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

Document how to use Flask-SQLAlchemy with MyPy #1327

Closed
fhd opened this issue Apr 22, 2024 · 2 comments
Closed

Document how to use Flask-SQLAlchemy with MyPy #1327

fhd opened this issue Apr 22, 2024 · 2 comments

Comments

@fhd
Copy link

fhd commented Apr 22, 2024

So, I'll admit this is a bit of a duplicate of #1186. The reason I'm writing this issue anyway is that it took me a couple of hours to finally find a decent workaround. Reddit, StackOverflow et al are filled with false positives that sent me on various wild goose chases. Only by manually going through the closed issues in this repo did I finally find #1186 and then subsequently the linked MyPy issue which had a workaround that works for me.

I really think it'd be very helpful for others to document how to use Flask-SQLAlchemy with MyPy. The question of whether this is a MyPy or a Flask-SQLAlchemy issue is a valid one, and I suppose it's more of a MyPy issue. But knowing that is not as helpful as useful workarounds.

If you (still) disagree, I wouldn't be offended if you just close this issue without a word. In fact, I rather hope I'm not offending @davidism by writing it - you must be tired of this topic :(

For anyone unfaimilar with the situation:

The problem

It seems there's quite a history of (vanilla) SQLAlchemy and MyPy not playing nicely together, but at least as of SQLAlchemy 2.0, it all seems to work out of the box. That history is probably why there's so many false positives (i.e. workarounds that don't work) out there when researching this issue.

For completeness sake, here's a way to reproduce it (I've used Flask-SQLAlchemy 3.1.1 and Python 3.9.19):

from flask_sqlalchemy import SQLAlchemy
from sqlalchemy import String
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column


class Base(DeclarativeBase):
    pass


db = SQLAlchemy(model_class=Base)


class Person(db.Model):
    __tablename__ = "person"

    id: Mapped[int] = mapped_column(primary_key=True)
    name: Mapped[str] = mapped_column(String(255))

    def __repr__(self) -> str:
        return f"Person(name={self.nme})"  # Deliberate typo here

In vanilla SQLAlchemy, Person would simply inherit Base, and that's alright with MyPy. In fact, it instead complains about the typo I've added (self.nme). That's the desired behaviour.

But with the above as-is, MyPy is not so happy:

error: Name "db.Model" is not defined  [name-defined]

I can see why: db.Model is set dynamically, not something MyPy can deal with.

Meet the workarounds

Ignoring the error

The most obvious workaround (and the one recommended in #1186) is to just disable the name-defined error for model files. One simple way to do that is to add # type: ignore[name-defined] after class Person(db.Model): (and any other model class).

The problem with this: It makes MyPy miss other errors. I added self.nme in the example code as a deliberate typo, and MyPy won't find it with name-defined ignored. Note that that's an attr-defined error - not a name-defined error. I can't even tell if this is a feature or a bug. The error also gets ignored when ignoring other errors at the class declaration level (see below). Assignment errors, on the other hand, still show up.

Perhaps this'll be solved in MyPy at some point (see MyPy issue 8603), but until then, it doesn't seem like the best workaround.

Declaring a type for db.Model

This one I'm just listing for completeness sake, since it's so widely spread online. For me, it doesn't work.

The idea is to declare your own Model type, assign db.Model to it, but give that a type that MyPy will be happy with as a base class. The oldest mentions do it like this:

from sqlalchemy.ext.declarative import DeclarativeMeta
...
Model: DeclarativeMeta = db.Model
...
class Person(Model):

MyPy doesn't buy it though:

error: Incompatible types in assignment (expression has type "type[_FSAModel]", variable has type "DeclarativeMeta")  [assignment]
error: Variable "foo.models.Model" is not valid as a type  [valid-type]
error: Invalid base class "Model"  [misc]

What I also found is people suggesting to use DefaultMeta from Flask-SQLAlchemy instead of DeclarativeMeta:

from flask_sqlalchemy.model import DefaultMeta
...
Model: DefaultMeta = db.Model

Gives me the exact same errors though.

So here I went, adding workarounds for the workaround. If I use typing.cast or # type: ignore[assignment] for the Model assignment, the first error is gone, leaving just the valid-type and misc errors where I use Model as a parent class. Now if I add # type: ignore[valid-type,misc] to tackle those, MyPy is happy and reports no issues. Again not catching my self.nme typo (which is an attr-defined error). So arguably this workaround is just more typing to the same effect as the first one (from what I can tell).

Using a different base class for type checking

A variant of this was mentioned in #1186 as the solution recommended by the reporter, and for me, it's the only one that does the trick:

from typing import TYPE_CHECKING
...
if TYPE_CHECKING:
    from flask_sqlalchemy.model import Model
else:
    Model = db.Model

With this, everything works and the self.nme typo is found by MyPy. I think it might bring trouble down the line, but at least for my code so far, I didn't find any issues.

What now?

I'd be happy to put some more time into this and try and figure out what the best workaround would be - or when to use which one perhaps - and to document it. But first I wanted to see if you're (now) OK with documenting it. If not, perhaps some poor soul like me will find this thread, open or closed, and might save a bit of time.

Update (2024-05-09)

Only now I noticed that the TYPE_CHECKING workaround seems to have the side effect that MyPy complains about any keyword arguments to models. If I assign Model to Base instead, that seems to work, but it'll make MyPy accept any keyword arguments, regardless of whether they exist or not. Personally, I've chosen to use the workaround as-is and to not use keyword arguments to models in type checked code. This way I'm forced to assign properties directly, and there MyPy does report when a property doesn't exist.

@davidism
Copy link
Member

davidism commented May 8, 2024

Declaring a type for db.Model

I use exactly what you've described, and it does work.

I don't particularly want to document any of these since this is an issue with MyPy and Pyright (or just the typing spec in general). Report issues there, contribute there to get fixes.

@fhd
Copy link
Author

fhd commented May 9, 2024

I understand that. I mean, at this point, I hope this issue is reasonably easy to find, and there's also a StackOverflow answer now. I'll just close it now, since leaving it open just leaves it on your mental list. If you don't mind, I'll update it here occasionally if I make any new discoveries. I'll also think about how this could perhaps be documented on the MyPy side.

@fhd fhd closed this as completed May 9, 2024
jtoledo1974 added a commit to jtoledo1974/atcapp that referenced this issue May 20, 2024
…eeming incompatible with mypy. It is explained in pallets-eco/flask-sqlalchemy#1327.  I've tried all approaches and this seems to me the simplest hack to get mypy happy. We don't get autocomplete for model methods like query, but at least we do get type checking of columns.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants