flask-sqlalchemy icon indicating copy to clipboard operation
flask-sqlalchemy copied to clipboard

Type error when calling db.Model subclass constructor with parameters

Open tkieft opened this issue 1 year ago • 7 comments

Pylance / Pyright reports a type error when instantiating a Model subclass with parameters:

Expected no arguments to "User" constructor

Minimal example:

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


class Base(DeclarativeBase):
    pass


db = SQLAlchemy(model_class=Base)


class User(db.Model):
    __tablename__ = "users"

    id: Mapped[int] = mapped_column(BigInteger, primary_key=True, autoincrement=True)
    username: Mapped[str] = mapped_column(String(255), unique=True, nullable=False)


def test():
    user = User(username="x")
    print(user)

I expected there to be no type errors, since this operation is supported within SQLAlchemy.

Environment:

  • Python version: 3.10.13
  • Flask-SQLAlchemy version: 3.1.1
  • SQLAlchemy version: 2.0.18

tkieft avatar Feb 09 '24 22:02 tkieft

I'm facing the same issue. Additionally, MappedAsDataclass to the base class does not work.

As a workaround, adding MappedAsDataclass after db.Model in the User class seems to work.

Alternatively:

class Base(DeclarativeBase, MappedAsDataclass):
    pass


class ProperlyTypedSQLAlchemy(SQLAlchemy):
    """Temporary type hinting workaround for Flask SQLAlchemy.

    This is a temporary workaround for the following issue:
    https://github.com/pallets-eco/flask-sqlalchemy/issues/1312
    This workaround may not be correct.
    """

    Model: Type[Base]


db = SQLAlchemy(model_class=Base)
db = cast(ProperlyTypedSQLAlchemy, db)

thecodingwizard avatar Mar 03 '24 22:03 thecodingwizard

This issue is related to Python, not Flasksqlalchmey.

Your class definition is missing a constructor to initialize it with arguments the way you are doing.

When you create a new object from a Class in Python you can't initialize it with arguments unless it has a constructor function that runs when a new instance is created from this class and initializes it with the values you are passing.

the correct code will be like this:

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


class Base(DeclarativeBase):
    pass


db = SQLAlchemy(model_class=Base)


class User(db.Model):
    __tablename__ = "users"

    id: Mapped[int] = mapped_column(BigInteger, primary_key=True, autoincrement=True)
    username: Mapped[str] = mapped_column(String(255), unique=True, nullable=False)

    def __init__(self, username):
        self.username = username


def test():
    user = User(username="x")
    print(user)

Also, note that you pass the Base as the model class to the SQLAlchemy instance, not the imported DeclarativeBase so you have the flexibility to customize the Base class and everything will inherit from it.

john0isaac avatar Mar 14 '24 09:03 john0isaac

I spot another typehint issue just now. See #1318

Hopefully, these two issues can be solved together. To my understanding. SQLAlchemy should be a generic class like this:

class SQLAlchemy(Generic[_FSA_MCT]):
    def __init__(..., model_class: _FSA_MCT, ...): ...

    @property
    def Model(self) -> _FSA_MCT: ...

In current implementation, the TypeVar has already been provided. However, SQLAlchemy it not generic, that's why the type check fails. https://github.com/pallets-eco/flask-sqlalchemy/blob/42a36a3cb604fd39d81d00b54ab3988bbd0ad184/src/flask_sqlalchemy/extension.py#L164-L175

If the typehints can be corrected, the dirty workaround # type: ignore[assignment] at Line 171 can be removed.

cainmagi avatar Mar 26 '24 16:03 cainmagi

Happy to review a PR

davidism avatar Mar 26 '24 17:03 davidism

@davidism

I am trying to make a PR now. However, I am sorry that this issue is far more complicated than I thought.

self.Model should not be merely the model_class passed in the initialization. Actually, in some cases, it is also a subclass of Model. I will try to give a solution, but it may not be able to totally solve this issue.

cainmagi avatar Mar 26 '24 17:03 cainmagi

I think I got an idea to fix it. But I am not sure whether this fixture will cause side effects. I am working on #1318 now. If it has been finalized, I will try to submit another PR for this.

from sqlalchemy.util import typing as compat_typing

@compat_typing.dataclass_transform(
    field_specifiers=(
        sa_orm.MappedColumn,
        sa_orm.RelationshipProperty,
        sa_orm.Composite,
        sa_orm.Synonym,
        sa_orm.mapped_column,
        sa_orm.relationship,
        sa_orm.composite,
        sa_orm.synonym,
        sa_orm.deferred,
    ),
)
class _FSAModel(Model):
    metadata: sa.MetaData

After changing this, I found the db.Model has correct initialization. image

But this change still needs some improvement. It should take effect only when provided model_class is subtype of MappedAsDataclass. Maybe I can add an overload to implement it.

cainmagi avatar Mar 26 '24 19:03 cainmagi

@davidism I am glad to tell you that I finally found a good solution (see #1321). Although my PR still has few remained issues. It has tackled the type errors in most cases. I also attach codes for testing is and attach another part for explaining my idea.

Even if you reject this PR, I still hope that it can suggest a possible direction for solving this issue. At least, this PR works well for me.

cainmagi avatar Mar 27 '24 22:03 cainmagi