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

pylance does not seem to understand customized descriptor __get__ method. #6472

Closed
raceychan opened this issue Sep 28, 2024 · 2 comments
Closed
Assignees

Comments

@raceychan
Copy link

raceychan commented Sep 28, 2024

import typing as ty

class ClassAttr[AttrType]:
    """
    Like the opposite of 'property',
    where this returns class attribute only if accssed via class
    and return itself otherwise.

    class Test:
        name: ClassAttr[str] = ClassAttr(lambda x: x.__name__.lower())

    assert Test.name == "test"
    assert isinstance(Test().name, ClassAttr)
    """
    def __init__(
        self,
        name_or_getter: str | ty.Callable[[type], AttrType],
        *,
        default: AttrType | _Missing = MISSING,
    ):
        self._name_or_getter = name_or_getter
        self._default = default

    def __set_name__(self, owner_type: type, name: str):
        self._set_name = name

    @ty.overload
    def __get__[T](self, owner_obj: T, owner_type: type[T]) -> ty.Self: ...

    @ty.overload
    def __get__[
        T
    ](self, owner_obj: ty.Literal[None], owner_type: type[T]) -> AttrType: ...

    def __get__[
        T
    ](self, owner_obj: T | ty.Literal[None], owner_type: type[T]) -> AttrType | ty.Self:
        if owner_obj is not None:
            return self
        if isinstance(self._name_or_getter, ty.Callable):
            _val = self._name_or_getter(owner_type)
        else:
            try:
                _val: AttrType = getattr(owner_type, self._name_or_getter)
            except AttributeError as ae:
                if self._default is not MISSING:
                    self._default = ty.cast(AttrType, self._default)
                    return self._default
                raise ae

        return _val


class T:
    name: ClassAttr[str] = ClassAttr[str](lambda cls: cls.__name__.lower())

    @property
    def age(self) -> int:
        return 0


T().name # (variable) name: ClassAttr[str]
T.name# (variable) name: ClassAttr[str]

The doc string should explain what this class does well,
this 'ClassAttr' is expected to return generic 'AttrType' when it is accessed via class object, and return itself when accesed via instance object.
however, pylance seems to think that it would always return ClassAttr either way.

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Sep 28, 2024
@raceychan raceychan changed the title pylance does not seem to understand descriptor. pylance does not seem to understand customized descriptor __get__ method. Sep 28, 2024
@erictraut
Copy link
Contributor

The problem here is that the type variable T in the first overload has no upper bound, so None is a valid value for the owner_obj parameter.

You can solve this problem by swapping the order of your overloads.

from typing import Any, ClassVar, Self, overload

class ClassAttr[AttrType]:
    def __init__(self): ...

    @overload
    def __get__(self, owner_obj: None, owner_type: type[Any]) -> AttrType: ...

    @overload
    def __get__[T](self, owner_obj: T, owner_type: type[T] | None = None) -> Self: ...

    def __get__[T](self, owner_obj: T | None, owner_type: type[T] | None = None) -> AttrType | Self: ...

class T:
    name: ClassVar[ClassAttr[str]]


x1 = T.name
reveal_type(x1)  # str

x2 = T().name
reveal_type(x2)  # ClassAttr[str]

I made a couple of other changes in the code sample that you might find instructive:

  1. There's no need to use Literal with None in a type annotation. The Literal is redundant in this case, since Literal[None] describes the same type as None.
  2. One of the two overloads doesn't require a type variable. In general, if you find that you are using a type variable only once in a signature, it's probably unnecessary.
  3. I changed the T.name attribute to be a ClassVar. This tells a type checker that it should not be overwritten by an instance variable of the same type. This is important in this case because the behavior of a descriptor object changes if it is assigned as an instance variable.
  4. I changed the type of the owner_type parameter on the second overload from type[T] to type[T] | None and added a default argument value of None. AFAIK, the descriptor protocol doesn't guarantee that this second argument is passed under all circumstances when the first argument is not None. For this reason, most descriptors are defined in this manner. See the code in typeshed's builtins.py stub as an example.

@debonte debonte added by design and removed needs repro Issue has not been reproduced yet labels Sep 28, 2024
@debonte debonte closed this as completed Sep 28, 2024
@raceychan
Copy link
Author

The problem here is that the type variable T in the first overload has no upper bound, so None is a valid value for the owner_obj parameter.

You can solve this problem by swapping the order of your overloads.

from typing import Any, ClassVar, Self, overload

class ClassAttr[AttrType]:
    def __init__(self): ...

    @overload
    def __get__(self, owner_obj: None, owner_type: type[Any]) -> AttrType: ...

    @overload
    def __get__[T](self, owner_obj: T, owner_type: type[T] | None = None) -> Self: ...

    def __get__[T](self, owner_obj: T | None, owner_type: type[T] | None = None) -> AttrType | Self: ...

class T:
    name: ClassVar[ClassAttr[str]]


x1 = T.name
reveal_type(x1)  # str

x2 = T().name
reveal_type(x2)  # ClassAttr[str]

I made a couple of other changes in the code sample that you might find instructive:

  1. There's no need to use Literal with None in a type annotation. The Literal is redundant in this case, since Literal[None] describes the same type as None.
  2. One of the two overloads doesn't require a type variable. In general, if you find that you are using a type variable only once in a signature, it's probably unnecessary.
  3. I changed the T.name attribute to be a ClassVar. This tells a type checker that it should not be overwritten by an instance variable of the same type. This is important in this case because the behavior of a descriptor object changes if it is assigned as an instance variable.
  4. I changed the type of the owner_type parameter on the second overload from type[T] to type[T] | None and added a default argument value of None. AFAIK, the descriptor protocol doesn't guarantee that this second argument is passed under all circumstances when the first argument is not None. For this reason, most descriptors are defined in this manner. See the code in typeshed's builtins.py stub as an example.

Absolutly astonishing! Thanks for your detailed and complete answer! never thought that the order of typing.overload would matter, TIL. Other tips are very helpful too!
Thanks, eric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants