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

Mapped property is interpreted to mapped type instead of InstrumentedAttibute on union #10751

Closed
2 tasks done
Fred-si opened this issue Dec 11, 2023 · 8 comments
Closed
2 tasks done
Labels
third party integration issues issues to do with other libraries and frameworks typing pep -484 typing issues. independent of "mypy"

Comments

@Fred-si
Copy link

Fred-si commented Dec 11, 2023

Ensure stubs packages are not installed

  • No sqlalchemy stub packages is installed (both sqlalchemy-stubs and sqlalchemy2-stubs are not compatible with v2)

Verify if the api is typed

  • The api is not in a module listed in #6810 so it should pass type checking

Describe the typing issue

When I have a variable that contain an union of model type (foo: type[ModelA | ModelB]) property type is interpreted by mypy as mapped_type instead of InstrumentedAttribute[mapped_type].

To Reproduce

from typing_extensions import reveal_type

from sqlalchemy.orm import DeclarativeBase, Mapped


class Foo(DeclarativeBase):
    foobar: Mapped[str]


class Bar(DeclarativeBase):
    foobar: Mapped[str]


def foo(attribute: InstrumentedAttribute[Any]) -> None:
    pass


union: type[Foo | Bar] = Foo
foo(union.foobar)

Error

main.py:21: error: Argument 1 to "foo" has incompatible type "str"; expected "InstrumentedAttribute[Any]"  [arg-type]

Versions

  • OS: Ubuntu 20.04
  • Python: 3.10.12 | 3.11.6 | 3.12.0
  • SQLAlchemy: 2.0.22 | 2.0.23
  • Type checker (eg: mypy 0.991, pyright 1.1.290, etc): mypy 1.7.1

Additional context

sqlalchemy and mypy are installed in venv

requirements.txt

SQLAlchemy==2.0.23
mypy==1.7.1

minimal example can be found here https://github.com/Fred-si/sqlalchemy_union_type_issue

@Fred-si Fred-si added requires triage New issue that requires categorization typing pep -484 typing issues. independent of "mypy" labels Dec 11, 2023
@zzzeek
Copy link
Member

zzzeek commented Dec 11, 2023

that's going to be a Mypy issue. If you replace InstrumentedAttribute with a plain Python descriptor you will likely get the same result.

I'll show you a confirming case in a few (will close this issue).

@zzzeek zzzeek added third party integration issues issues to do with other libraries and frameworks external library/application issues a separate library / application that's not SQLAlchemy has a problem (dependent or dependee) and removed requires triage New issue that requires categorization external library/application issues a separate library / application that's not SQLAlchemy has a problem (dependent or dependee) labels Dec 11, 2023
@zzzeek
Copy link
Member

zzzeek commented Dec 11, 2023

adding reveal_type() to your example, here's mypy:

$ mypy test3.py 
lib/sqlalchemy/util/concurrency.py:84: error: Cannot find implementation or library stub for module named "greenlet"  [import-not-found]
lib/sqlalchemy/util/concurrency.py:84: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
test3.py:19: note: Revealed type is "builtins.str"
test3.py:21: error: Argument 1 to "foo" has incompatible type "str"; expected "InstrumentedAttribute[Any]"  [arg-type]
Found 2 errors in 2 files (checked 1 source file)

however here's pyright

$ pyright test3.py 
/home/classic/dev/sqlalchemy/test3.py
  /home/classic/dev/sqlalchemy/test3.py:19:13 - information: Type of "union.foobar" is "InstrumentedAttribute[str]"
0 errors, 0 warnings, 1 information 

pyright is correct, mypy is wrong. mypy has a lot of problems dealing with Type objects in a meaningful way.

Here's a non-SQLAlchemy example, feel free to report this to mypy:

from __future__ import annotations

from typing import Any, overload, Generic, TypeVar, Optional, Union

_T_co = TypeVar("_T_co", bound=Any, covariant=True)


class InstrumentedAttribute(Generic[_T_co]):
    @overload
    def __get__(
        self, instance: None, owner: Any
    ) -> InstrumentedAttribute[_T_co]:
        ...

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

    def __get__(
        self, instance: Optional[object], owner: Any
    ) -> Union[InstrumentedAttribute[_T_co], _T_co]:
        ...

    def __set__(self, instance: Any, value: Any) -> None:
        ...

    def __delete__(self, instance: Any) -> None:
        ...


class Foo:
    foobar: InstrumentedAttribute[str]


class Bar:
    foobar: InstrumentedAttribute[str]


def foo(attribute: InstrumentedAttribute[Any]) -> None:
    pass


union: type[Foo | Bar] = Foo

reveal_type(union.foobar)

foo(union.foobar)

reproduces in the same way as the previous example for both tools (correct for pyright, wrong for mypy)

@zzzeek zzzeek closed this as completed Dec 11, 2023
@CaselIT
Copy link
Member

CaselIT commented Dec 11, 2023

This is issue python/mypy#5570 of mypy

Considering it's open since 2018 I'm not sure how soon it will be fixed

@Fred-si
Copy link
Author

Fred-si commented Dec 11, 2023

Thanks for yours replies. My apologies for the mistake.

@CaselIT
Copy link
Member

CaselIT commented Dec 11, 2023

No problem!

@md384
Copy link

md384 commented Jan 21, 2024

This will hopefully be fixed by python/mypy#16604

@CaselIT
Copy link
Member

CaselIT commented Jan 22, 2024

thanks for reporting back. No change is required in sqlalchemy, correct?

@md384
Copy link

md384 commented Jan 22, 2024

thanks for reporting back. No change is required in sqlalchemy, correct?

Correct - it appears to be a bug only in mypy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
third party integration issues issues to do with other libraries and frameworks typing pep -484 typing issues. independent of "mypy"
Projects
None yet
Development

No branches or pull requests

4 participants