sqlalchemy / sqlalchemy2-stubs

PEP-484 typing stubs for SQLAlchemy 1.4
MIT License
159 stars 41 forks source link

Cannot infer type of "Column" when server_default and default kwargs are set on a @declared_attr #184

Open Davidonium opened 3 years ago

Davidonium commented 3 years ago

Describe the bug When adding the server_default kwarg to a Column declarative_attr, mypy complains like this:

error: Cannot infer type argument 1 of "Column"

not really sure if I am doing something wrong but I am doing as stated here: https://docs.sqlalchemy.org/en/14/core/metadata.html?highlight=server_default#sqlalchemy.schema.Column.params.server_default

Expected behavior mypy to not error

To Reproduce This snippet should reproduce the error:

import sqlalchemy as sa
from sqlalchemy.orm import (
    Mapped,
    declarative_mixin,
    declared_attr,
)
from datetime import datetime

@declarative_mixin
class AuditMixin:
    @declared_attr
    def created_at(self) -> Mapped[datetime]:
        return sa.Column(
            sa.DateTime(timezone=True),
            server_default=sa.func.now(), # this is the culprit, commenting this line makes mypy pass
            default=datetime.utcnow,
            nullable=False,
        )

    @declared_attr
    def updated_at(self) -> Mapped[Optional[datetime]]:
        return sa.Column(
            sa.DateTime(timezone=True),
            onupdate=datetime.utcnow,
        )

Then run mypy with sqlalchemy2-stubs on this file.

Error

error: Cannot infer type argument 1 of "Column"

Versions.

Many thanks in advance.

CaselIT commented 3 years ago

Hi,

Thanks for reporting. Just a guess. removing default=datetime.utcnow, improve things? Setting default when using server_default is probably not the very usual, since default would be used by sqlalchemy instead of server_default

In any case this is probably a bug in the stubs.

Davidonium commented 3 years ago

After trying, it does work so I guess that the bug is when setting both server_default and default at the same time. Setting only one of them makes mypy pass, so I guess this is rather strange to encounter (I did this from lack of experience with sqlalchemy). I'll change the description.

CaselIT commented 3 years ago

Thanks for testing that change.

Settings both is supported, even if unusual, I think, so we should try to resolve the issue.

@zzzeek can you confirm that we support this case?