Skip to content
New issue

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

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

Already on GitHub? # to your account

Property accessor hack for SQLAlchemy columns #444

Closed
layday opened this issue Dec 21, 2019 · 5 comments
Closed

Property accessor hack for SQLAlchemy columns #444

layday opened this issue Dec 21, 2019 · 5 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@layday
Copy link

layday commented Dec 21, 2019

Describe the bug
The SQLAlchemy stubs from Dropbox use descriptors for Column attributes returning the underlying type (in this case, a string) for objects of the enclosing class [2]:

class Column(SchemaItem, ColumnClause[_T]):
    ...
    @overload
    def __get__(self, instance: None, owner: Any) -> Column[_T]: ...
    @overload
    def __get__(self, instance: object, owner: Any) -> _T: ...

This is not being picked up by pyright. Compare with mypy:

Screenshot 2019-12-21 at 03 50 58

To Reproduce
Add the SQLAlchemy stubs in typingsPath and type check the snippet below.

Expected behavior
Foo().bar should be of type str.

Screenshots or Code

from sqlalchemy import Column, String


class Foo:
    bar = Column(String)


reveal_type(Foo.bar)
reveal_type(Foo().bar)

VS Code extension or command-line
extension, 1.1.11

@erictraut
Copy link
Collaborator

Thanks for the bug report. Pyright wasn't properly handling overloads for the __get__ method. This will be fixed in the next version of Pyright.

@erictraut erictraut added bug Something isn't working addressed in next version Issue is fixed and will appear in next published version labels Dec 21, 2019
@erictraut
Copy link
Collaborator

This is now fixed in version 1.1.12, which I just published.

@nourselim0
Copy link

nourselim0 commented Sep 18, 2020

@erictraut This is very similar to the issue I'm facing (overloads on __get__ to achieve accurate ORM types) but with Django Stubs instead of SQLAlchemy. In my case Pylance (or Pyright) says the type is Any instead of str.
Here is the relevant part in the Django Stubs repo: https://github.com/typeddjango/django-stubs/blob/master/django-stubs/db/models/fields/__init__.pyi#L107

Either this issue has regressed, or there is a bug in the django stubs package. Honestly python generics and overloads are still relatively new to me and I struggle with deciphering them, so any help is appreciated.

Update: Its seems like the issue is in django-stubs, I think they are keeping all field types generic without specifying a concrete type for the generic parameter. Here is CharField for example: https://github.com/typeddjango/django-stubs/blob/master/django-stubs/db/models/fields/__init__.pyi#L199
Locally I updated class CharField(Field[_ST, _GT]): to be class CharField(Field[str, str]): and it fixed the issue. However I feel like I might have broken something else by this change, it's so simple the package authors probably thought of that didn't do it for a reason I don't know. I guess it's time for me to take that issue over to django-stubs issue tracker 😅

Update 2: I figure it out, django-stubs has a custom mypy plugin that parses a _pyi_private_get_type field that specifies the concrete type of the __get__ on each concrete field (similar thing for __set__). I assume there is no way to make that mypy plugin to work with Pyright. So yeah, I guess I should have that discussion with the stub contributors.

@erictraut
Copy link
Collaborator

I did find a bug in Pyright. it was not properly handling the __get__ methods with generic types. I don't know if this will completely fix the issue at hand.

You are correct that django-stubs relies on a mypy plugin for some of its behavior. Mypy plugins are very specific to mypy. They don't work with other type checkers like pyright.

@nourselim0
Copy link

Yeah I'm almost certain that django-stubs was made in such a way that orm field type would only work on mypy, that _pyi_private_get_type attr is not a standard and I found code inside a mypy plugin folder that parses it.

But it's good that you found that bug, even if it wont solve my problem.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants