SQLAlchemy stubs produce errors with basic/typical usage

[Deimos]

Originally posted on the mypy repo, copying my example over to here.

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.

Mentioning @WouldYouKindly since he did the major update in #857.

[garetht]

I think this might have something to do with the limitation discussed in this issue.

[gvanrossum]

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).

[garetht]

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.

[gvanrossum]

Well, full static type checking of code involving such magic is just not in the cards.

[lincolnq]

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.

[Deimos]

@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).

[gvanrossum]

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]

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:

from sqlalchemy import Column, String
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class User(Base):
    __tablename__ = 'users'

    name: str = Column(String)  # type: ignore

    def __init__(self, name: str) -> None:
        self.name = name

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]

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]

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 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. 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.
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.

[gvanrossum]

I'd prefer to see if we (you :-) can fix the stubgen crashes.

…

--Guido (mobile)

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> .

[JelleZijlstra]

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.

[JelleZijlstra]

I just submitted more complete stubs in #1094. Hopefully that will fix this issue.

[JelleZijlstra]

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]

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]

@j-walker23
mypy supports descriptors. Also SQLAlchemy stubs now moved to https://github.com/dropbox/sqlalchemy-stubs/

[j-walker23]

@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!