MarshalX / atproto

The AT Protocol (🦋 Bluesky) SDK for Python 🐍
https://atproto.blue
MIT License
267 stars 29 forks source link

upgrade is_record_type to use TypeGuard #123

Open DXsmiley opened 11 months ago

DXsmiley commented 11 months ago

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

https://github.com/MarshalX/atproto/blob/d5e2293848a3506a1b13144062c24bae1cc10f2c/atproto/xrpc_client/models/utils.py#L202

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 commented 11 months ago

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 commented 9 months ago

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.