Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrade is_record_type to use TypeGuard #123

Open
DXsmiley opened this issue Aug 3, 2023 · 2 comments
Open

upgrade is_record_type to use TypeGuard #123

DXsmiley opened this issue Aug 3, 2023 · 2 comments
Labels
good first issue Good for newcomers

Comments

@DXsmiley
Copy link
Contributor

DXsmiley commented Aug 3, 2023

Would it be possible to utilise TypeGuard on this function in order to leverage the return value at the callsite for static type checking?

def is_record_type(model: ModelBase, expected_type: t.Union[str, types.ModuleType]) -> bool:

I'd do it myself but I don't feel I understand enough about the details of the library at this point to get it right

@MarshalX MarshalX added the good first issue Good for newcomers label Aug 28, 2023
@MarshalX
Copy link
Owner

MarshalX commented Aug 28, 2023

sorry for the long response. I'll take a look after releasing #127

mypy is not fully integrated into atproto sdk :( but i have a plan for it

https://github.com/MarshalX/atproto/blob/main/pyproject.toml#L78-L80

@DXsmiley
Copy link
Contributor Author

DXsmiley commented Oct 2, 2023

Making a note so nobody else falls over the same problem, hopefully:

I spent a while banging my head against the wall on this one only to find that there was a bug in pyright which only recently got fixed.

Specifically:

Bug Fix: Fixed several issues with logic that performs protocol matching against a module, including a false positive error when matching against a generic protocol.

Here's my current (incomplete) approach, if anyone's interested:

from typing import TypeVar, Protocol, Any, Type
from typing_extensions import TypeGuard
from atproto.xrpc_client.models.base import ModelBase
from atproto.xrpc_client.models.utils import is_record_type as original_is_record_type
from atproto.xrpc_client import models

T = TypeVar('T', bound=ModelBase)

class HasAMainModel(Protocol[T]):
    Main: Type[T]

def is_record_type(record: Any, model: HasAMainModel[T]) -> TypeGuard[T]:
    return original_is_record_type(record, model)  # type: ignore

def test(x: Any):
    if is_record_type(x, models.AppBskyEmbedImages):
        reveal_type(x) # correctly identified as atproto.xrpc_client.models.app.bsky.embed.images.Main

Also need to consider that this actually isn't quite the same as an is_instance check so we should be careful not to assert too much about the type via the TypeGuard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants