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

classes with descriptor properties in Unions #5570

Closed
srittau opened this issue Sep 4, 2018 · 6 comments · Fixed by #17381
Closed

classes with descriptor properties in Unions #5570

srittau opened this issue Sep 4, 2018 · 6 comments · Fixed by #17381
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-descriptors Properties, class vs. instance attributes topic-union-types

Comments

@srittau
Copy link
Contributor

srittau commented Sep 4, 2018

Please consider:

from typing import Any, Union

class getter:
    def __get__(self, instance: "X1", owner: Any) -> str:
        return ""

class X1:
    prop = getter()

class X2:
    def __init__(self) -> None:
        self.prop = ""

def foo(x: Union[X1, X2]) -> None:
    x.prop  # error: Argument 1 to "__get__" of "getter" has incompatible type "Union[X1, X2]"; expected "X1"

mypy 0.620 (Python 3.6.6, default options) will warn:

foo.py:15: error: Argument 1 to "__get__" of "getter" has incompatible type "Union[X1, X2]"; expected "X1"

Changing the annotation of instance to Any will work around this problem.

@ilevkivskyi ilevkivskyi added bug mypy got something wrong priority-1-normal topic-union-types false-positive mypy gave an error on correct code labels Sep 4, 2018
@ilevkivskyi
Copy link
Member

Note there is even more subtle manifestation of this bug if overloads are involved:

class getter:
    @overload
    def __get__(self, instance: None, owner: Any) -> getter: ...
    @overload
    def __get__(self, instance: object, owner: Any) -> str: ...

class X1:
    prop = getter()
class X2:
    prop = getter()

def foo(x: Type[Union[X1, X2]]) -> None:
    reveal_type(x.prop)

This shows str instead of getter because a wrong type is passed for instance (same cause as in the original example). The fix may involve some refactoring for how attributes on unions are accessed.

@daisylb
Copy link

daisylb commented Feb 3, 2022

FWIW, I'm running into this exact same issue on 0.931, because django-stubs uses the instance: None trick on Fields to model descriptors that return themselves when not accessed on an instance.

Another short reproducible example, based on the django-stubs code:
from typing import Type, Union, Generic, TypeVar, overload

T = TypeVar("T")


class Model: ...

_T = TypeVar("_T", bound="Field")
_GT = TypeVar("_GT", covariant=True)

class Field(Generic[_GT]):
    @overload
    def __get__(self: _T, instance: None, owner) -> _T: ...
    # Model instance access
    @overload
    def __get__(self, instance: Model, owner) -> _GT: ...
    # non-Model instances
    @overload
    def __get__(self: _T, instance, owner) -> _T: ...


class A:
    attr: str


class B(Model):
    attr: Field[str] = Field()

b_inst: B
a_or_b_inst: Union[A, B]

reveal_type(B.attr) # test.Field[builtins.str]
reveal_type(b_inst.attr) # builtins.str*
reveal_type(a_or_b_inst.attr) # Union[builtins.str, test.Field[builtins.str]]
                              # (should just be builtins.str)

@JelleZijlstra JelleZijlstra added the topic-descriptors Properties, class vs. instance attributes label Mar 19, 2022
@sjschaff
Copy link

sjschaff commented Mar 3, 2023

Running into this issue as well with 1.0.1 and SQLAlchemy: Mypy type errors when using union's of model types #9419

Another, slightly different, repro.
from __future__ import annotations

from typing import Any
from typing import Generic
from typing import Literal
from typing import overload
from typing import Type
from typing import TYPE_CHECKING
from typing import TypeVar
from typing import Union

_T = TypeVar("_T", bound=Any)


class SQLWidgetThing(Generic[_T]):
  pass


class MyDescriptor(Generic[_T]):
  if TYPE_CHECKING:

      @overload
      def __get__(
          self, instance: Any, owner: Literal[None]
      ) -> MyDescriptor[_T]:
          ...

      @overload
      def __get__(
          self, instance: Literal[None], owner: Any
      ) -> SQLWidgetThing[_T]:
          ...

      @overload
      def __get__(self, instance: object, owner: Any) -> _T:
          ...

      def __get__(
          self, instance: object, owner: Any
      ) -> Union[MyDescriptor[_T], SQLWidgetThing[_T], _T]:
          ...


class Base:
  pass


class Model1(Base):
  my_attribute: MyDescriptor[str]


class Model2(Base):
  my_attribute: MyDescriptor[str]


ModelUnionTypeApproachA = Union[Type[Model1], Type[Model2]]
ModelUnionTypeApproachB = Union[Model1, Model2]

some_model_1: Type[Model1] = Model1
some_model_2: Type[Model2] = Model2
some_model_3: ModelUnionTypeApproachA = Model1
some_model_4: Type[ModelUnionTypeApproachB] = Model2

reveal_type(some_model_1.my_attribute)
reveal_type(some_model_2.my_attribute)
reveal_type(some_model_3.my_attribute)
reveal_type(some_model_4.my_attribute)

mypy:

$ mypy test4.py 
test4.py:64: note: Revealed type is "test4.SQLWidgetThing[builtins.str]"
test4.py:65: note: Revealed type is "test4.SQLWidgetThing[builtins.str]"
test4.py:66: note: Revealed type is "builtins.str"
test4.py:67: note: Revealed type is "builtins.str"
Success: no issues found in 1 source file

pyright:

$ pyright test4.py 
pyright 1.1.296
/home/classic/dev/sqlalchemy/test4.py
  /home/classic/dev/sqlalchemy/test4.py:64:13 - information: Type of "some_model_1.my_attribute" is "SQLWidgetThing[str]"
  /home/classic/dev/sqlalchemy/test4.py:65:13 - information: Type of "some_model_2.my_attribute" is "SQLWidgetThing[str]"
  /home/classic/dev/sqlalchemy/test4.py:66:13 - information: Type of "some_model_3.my_attribute" is "SQLWidgetThing[str]"
  /home/classic/dev/sqlalchemy/test4.py:67:13 - information: Type of "some_model_4.my_attribute" is "SQLWidgetThing[str]"
0 errors, 0 warnings, 4 informations 
Completed in 0.705sec

@DavideCanton
Copy link

Same error with mypy 1.1.1 and python 3.10:

from typing import Any, overload

from typing_extensions import reveal_type


class Column:
    def foo(self) -> int:
        return 42


class Descriptor:
    @overload
    def __get__(self, instance: None, owner: Any) -> Column:
        ...

    @overload
    def __get__(self, instance: object, owner: Any) -> int:
        ...

    def __get__(self, instance: object | None, owner: Any) -> Column | int:
        ...


class Model1:
    prop: Descriptor


class Model2:
    prop: Descriptor


def func1(m: type[Model1 | Model2]):
    reveal_type(m.prop)  # revealed type is int -> wrong, expected Column
    m.prop.foo()  # int has no attribute foo -> wrong, d.x is not an int


def func2(m: type[Model1]):
    reveal_type(m.prop)  # revealed type is Column -> correct
    reveal_type(m.prop.foo())  # revealed type is int -> correct

asottile-sentry added a commit to getsentry/sentry that referenced this issue Jul 21, 2023
working around python/mypy#5570

this is especially important as more things are being moved to unions of
`HC | Model`
armenzg pushed a commit to getsentry/sentry that referenced this issue Jul 24, 2023
working around python/mypy#5570

this is especially important as more things are being moved to unions of
`HC | Model`
@nielsbuwen
Copy link

Hi, any news on this? I have a similar case, where the descriptor can only work inside specific classes:

from typing import overload, Any


class BaseX:
    pass


class getter:
    @overload
    def __get__(self, instance: None, owner: Any) -> 'getter':
        ...

    @overload
    def __get__(self, instance: BaseX, owner: Any) -> str:
        ...

    def __get__(self, instance: BaseX | None, owner: Any) -> 'getter | str':
        return self


class X1(BaseX):
    prop = getter()


class X2(BaseX):
    prop = getter()


def foo(x: type[X1 | X2]) -> None:
    reveal_type(x.prop)

When foo accepts a union x.prop is illegal and shows a strange error message

No overload variant of "get" of "getter" matches argument types "type[X1] | type[X2]", "type[type[X1]] | type[type[X2]]"

There is an additional layer of type around the classes. Without the union, everything works.

Stats:

  • python: 3.11
  • mypy 1.5.1 (compiled: yes)

asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Nov 9, 2023
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Nov 9, 2023
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Nov 9, 2023
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Nov 9, 2023
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Dec 5, 2023
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
@Lacrymology
Copy link

I'm running into the same thing with alchemy except instead of having a Union of models, I've got a protocol with a Union for a type:

class Entity(Protocol):
    id: Union[int, str, Mapped[int], Mapped[str]]


# FIXME: this was supposed to receive entity: Entity, but mypy is complaining with
# foo.py:218:26: error: Argument 1 to "EntityKey" has incompatible type "User"; expected "Entity"  [arg-type]
# foo.py: note: Following member(s) of "PickWallToReplenishmentArea" have conflicts:
# foo.py: note:     id: expected "Union[int, str, Mapped[int], Mapped[str]]", got "Mapped[int]"
#
# Ideally we'd even be able to remove Mapped from the definition, but can't even make it work with it, so
# I decided to leave it there just to show I tried.
def EntityKey(entity) -> Key:
    model_name = entity.__table__.name if hasattr(entity, "__table__") else entity.__class__.__name__
    id = f"{model_name.lower()}_{entity.id}"

    return Key(id=id)

asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue Apr 30, 2024
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this issue May 28, 2024
(not upstreamed)

hopefully eventually can remove when python/mypy#5570 is solved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal topic-descriptors Properties, class vs. instance attributes topic-union-types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants