You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SQLAlchemy stubs produce errors with basic/typical usage #974
Here's an example of a really straightforward usage of SQLAlchemy (almost straight out of the beginning of the ORM tutorial) that's now producing mypy errors:
from sqlalchemy import Column, String
from sqlalchemy.ext.declarative import declarative_base
Base = declarative_base()
class User(Base):
__tablename__ = 'users'
name = Column(String)
def __init__(self, name: str) -> None:
self.name = name
The error is:
error: Incompatible types in assignment (expression has type "Column", variable has type "str")
So it seems like the stubs aren't setting up Column types as compatible with the basic types, which produces errors whenever they're assigned to, used as arguments to functions, etc.
I really suspect that the solution is just to add a fake __get__ method to the Column class (and to all other similar classes, maybe in an appropriate base class).
Hmm that might work here but SQLAlchemy also needs to support expressions like User.name.in_(['test']). __get__ can't return a str on the class since strings don't have in_ but it must return a str on the instance. Although I would be really, really happy to be misunderstanding something and for this to be possible.
Ok, this issue is causing my codebase to start failing type-checking in mypy v501 with hundreds of errors, for code which passes in v471. It breaks in a way which is not very simple to fix or work around. Almost none of these errors are real type errors in my code -- they're all issues with incomplete sqlalchemy stubs. And I think this will be true for a ton of codebases out there because, as noted in this report, the currently committed stubs fail type checking on common use cases.
I'd appreciate suggestions for workarounds -- I've tried some things which didn't work and so I'm a bit stuck:
I don't think there's a module specific way to blacklist usage of stubfiles (I would now blacklist all of sqlalchemy if I could, allowing its types to be inferred as Any as though stub files didn't exist).
I tried to regenerate stubs for all of sqlalchemy using stubgen, but stubgen crashes on 3 files in the sqlalchemy codebase that I have (1.1.3); and mypy crashes when type-checking the remaining stubs that were successfully generated. (note: it was even a bit tricky to figure out how to invoke stubgen on the whole sqlalchemy package contents; 'stubgen sqlalchemy' only generates override stubs for the root package and so it is still falling-back to the stubs found in typeshed for most imports.)
My current plan is to stay on mypy v471 until a more permissive set of stubs for sqlalchemy are released.
@lincolnq Personally, my "solution" for now is to just delete the sqlalchemy stubs. They're located in <base venv dir>/lib/mypy/typeshed/third_party/2and3/, and then I just completely deleted the sqlalchemy/ dir out of there.
But in my opinion, the stubs should probably be removed from typeshed when they're this broken, it doesn't do anyone any good to be shipping them if they can't handle the typical way the library is used. SQLAlchemy is an extremely popular library, and I think these stubs could scare off a lot of new people trying mypy if they try to implement it and see this many errors immediately (especially since they won't realize that you can't even really fix these errors).
Yeah, this is a serious issue. I think @Deimos' solution is the most reasonable work-around.
For mypy release 0.510 (due in about a month) we should come up with a better solution, either fixing the vast majority of issues that aren't real issues or remove the stubs.
I'm calling out @WouldYouKindly (the author of the new stubs in #857) and @ambv (who reviewed it a bit), as well as @timabbott since IIRC Zulip is also using SQLAlchemy and I'm curious about his experience with the new stubs. (I have no experience with it at all so I'm of relatively little help personally, sorry.)
@lincolnq Could you post more details about the issues you are finding? Is your codebase open source? It would really help knowing what you are running into, in order to determine whether we can fix this or whether a tactical retreat is advisable.
The reason it didn't fail in 0.47x is that the stubs were just not there. They're still super fragmentary (your example for instance imports .ext.declarative which is not yet stubbed).
The general issue with metaclasses like this (shared with attrs and Django models for example) is that mypy doesn't understand what's going to happen at runtime, and specifically, doesn't understand that class-level members get translated to incompatible instance-level members. Mypy has special handling for enum types for the same reason.
The current workaround in our internal codebase is to type hint it like this:
It's not ideal because you're technically "lying" to the interpreter about the type on the class, when you only mean to instruct it about the type on the instance. But it's obvious to read by humans so we live with it.
FWIW I'm fine with moving the stubs from typeshed to their separate repo (what Django does for example) if we don't improve the situation in time for 0.510.
What should the next steps be here? It sounds like there's a couple of different problems:
stubgen crashes when run on SQLAlchemy and sometimes creates stubs that crash mypy. Both of these sound like mypy bugs; @lincolnq can you file those bugs?
The current SQLAlchemy stubs are very incomplete, which means that people who use mypy on code that imports unstubbed bits of SQLAlchemy are going to get false positive errors. Added SQLAlchemy annotations #1029 is adding some missing stuff, but until we can get stubgen fixed or somebody takes the time to ensure all stubs files are complete, we should perhaps delete the existing stubs.
Some SQLAlchemy code involving descriptors doesn't fit the type system. Is there some way to make the stubs permissive enough to make those errors go away? If not, again that's a reason to delete at least some of the stubs.
On Mar 20, 2017 10:47 PM, "Jelle Zijlstra" ***@***.***> wrote:
What should the next steps be here? It sounds like there's a couple of
different problems:
1. stubgen crashes when run on SQLAlchemy and sometimes creates stubs
that crash mypy. Both of these sound like mypy bugs; @lincolnq
<https://github.com/lincolnq> can you file those bugs?
2. The current SQLAlchemy stubs are very incomplete, which means that
people who use mypy on code that imports unstubbed bits of SQLAlchemy are
going to get false positive errors. #1029
<#1029> is adding some missing
stuff, but until we can get stubgen fixed or somebody takes the time to
ensure all stubs files are complete, we should perhaps delete the existing
stubs.
3. Some SQLAlchemy code involving descriptors doesn't fit the type
system. Is there some way to make the stubs permissive enough to make those
errors go away? If not, again that's a reason to delete at least some of
the stubs.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#974 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACwrMjSDuDUXt_re5P8GWDEwMI5jXuyUks5rn2RYgaJpZM4MU9Cq>
.
I got stubgen to generate stubs for me by making it not crash if a module can't be imported. However, the generated stubs cause lots of errors when checked by mypy. I'll see how far I get fixing them.
Quick question please. Has mypy gained the ability to handle these types of descriptors since this ticket was closed?
I have the stubs and am trying different things. But i figured i'd ask if sqlalchemy's column attribute magic is even possible yet to type hint correctly.
Activity
garetht commentedon Mar 15, 2017
I think this might have something to do with the limitation discussed in this issue.
gvanrossum commentedon Mar 15, 2017
I really suspect that the solution is just to add a fake
__get__method to theColumnclass (and to all other similar classes, maybe in an appropriate base class).garetht commentedon Mar 15, 2017
Hmm that might work here but SQLAlchemy also needs to support expressions like
User.name.in_(['test']).__get__can't return astron the class since strings don't havein_but it must return astron the instance. Although I would be really, really happy to be misunderstanding something and for this to be possible.gvanrossum commentedon Mar 15, 2017
lincolnq commentedon Mar 15, 2017
Ok, this issue is causing my codebase to start failing type-checking in mypy v501 with hundreds of errors, for code which passes in v471. It breaks in a way which is not very simple to fix or work around. Almost none of these errors are real type errors in my code -- they're all issues with incomplete sqlalchemy stubs. And I think this will be true for a ton of codebases out there because, as noted in this report, the currently committed stubs fail type checking on common use cases.
I'd appreciate suggestions for workarounds -- I've tried some things which didn't work and so I'm a bit stuck:
My current plan is to stay on mypy v471 until a more permissive set of stubs for sqlalchemy are released.
Deimos commentedon Mar 15, 2017
@lincolnq Personally, my "solution" for now is to just delete the sqlalchemy stubs. They're located in
<base venv dir>/lib/mypy/typeshed/third_party/2and3/, and then I just completely deleted thesqlalchemy/dir out of there.But in my opinion, the stubs should probably be removed from typeshed when they're this broken, it doesn't do anyone any good to be shipping them if they can't handle the typical way the library is used. SQLAlchemy is an extremely popular library, and I think these stubs could scare off a lot of new people trying mypy if they try to implement it and see this many errors immediately (especially since they won't realize that you can't even really fix these errors).
gvanrossum commentedon Mar 15, 2017
Yeah, this is a serious issue. I think @Deimos' solution is the most reasonable work-around.
For mypy release 0.510 (due in about a month) we should come up with a better solution, either fixing the vast majority of issues that aren't real issues or remove the stubs.
I'm calling out @WouldYouKindly (the author of the new stubs in #857) and @ambv (who reviewed it a bit), as well as @timabbott since IIRC Zulip is also using SQLAlchemy and I'm curious about his experience with the new stubs. (I have no experience with it at all so I'm of relatively little help personally, sorry.)
@lincolnq Could you post more details about the issues you are finding? Is your codebase open source? It would really help knowing what you are running into, in order to determine whether we can fix this or whether a tactical retreat is advisable.
ambv commentedon Mar 15, 2017
The reason it didn't fail in 0.47x is that the stubs were just not there. They're still super fragmentary (your example for instance imports
.ext.declarativewhich is not yet stubbed).The general issue with metaclasses like this (shared with
attrsand Django models for example) is that mypy doesn't understand what's going to happen at runtime, and specifically, doesn't understand that class-level members get translated to incompatible instance-level members. Mypy has special handling forenumtypes for the same reason.The current workaround in our internal codebase is to type hint it like this:
It's not ideal because you're technically "lying" to the interpreter about the type on the class, when you only mean to instruct it about the type on the instance. But it's obvious to read by humans so we live with it.
FWIW I'm fine with moving the stubs from typeshed to their separate repo (what Django does for example) if we don't improve the situation in time for 0.510.
timabbott commentedon Mar 15, 2017
Zulip is using Django, not SQLAlchemy, for our models; we just use a small part of SQLAlchemy for generating our most complex database queries.
We have a couple
type: ignores related to the sqlalchemy stubs, but just a couple.JelleZijlstra commentedon Mar 21, 2017
What should the next steps be here? It sounds like there's a couple of different problems:
gvanrossum commentedon Mar 21, 2017
JelleZijlstra commentedon Mar 23, 2017
I got stubgen to generate stubs for me by making it not crash if a module can't be imported. However, the generated stubs cause lots of errors when checked by mypy. I'll see how far I get fixing them.
sqlalchemy: stubgen-generated stubs
JelleZijlstra commentedon Mar 25, 2017
I just submitted more complete stubs in #1094. Hopefully that will fix this issue.
JelleZijlstra commentedon Mar 28, 2017
We decided instead to remove the stubs completely (#1105). For those interested, I moved the incomplete SQLAlchemy stubs to a separate repo https://github.com/JelleZijlstra/sqlalchemy-stubs.
That means this typeshed issue can be closed.
j-walker23 commentedon Nov 16, 2018
Quick question please. Has mypy gained the ability to handle these types of descriptors since this ticket was closed?
I have the stubs and am trying different things. But i figured i'd ask if sqlalchemy's column attribute magic is even possible yet to type hint correctly.
ilevkivskyi commentedon Nov 16, 2018
@j-walker23
mypy supports descriptors. Also SQLAlchemy stubs now moved to https://github.com/dropbox/sqlalchemy-stubs/
j-walker23 commentedon Nov 16, 2018
@ilevkivskyi That's amazing! I did not know about the dropbox repo of sqlalchemy stubs. I will move off of the stubs at https://github.com/JelleZijlstra/sqlalchemy-stubs.
Thanks so much!
prepare release 4.0.1 (python#974)