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

Make Column and relationship inherit from Mapped #235

Open
mehdigmira opened this issue Oct 9, 2022 · 5 comments
Open

Make Column and relationship inherit from Mapped #235

mehdigmira opened this issue Oct 9, 2022 · 5 comments
Labels
bug Something isn't working missing type something is missing from the types

Comments

@mehdigmira
Copy link
Contributor

Is your feature request related to a problem? Please describe.

A typical typed SQLAlchemy model looks like this:

class Model(Base):
    name: Mapped[str] = Column(Unicode(100), nullable=True)

This works fine with mypy, but triggers the following error with Pylance/Pyright

Expression of type "Column[Unicode]" cannot be assigned to declared type "Mapped[str]"

Same for relationship

Describe the solution you'd like

Make Column and relationship inherit from Mapped

Additional context

I can make a PR if you're okay with this.

Have a nice day!

@mehdigmira mehdigmira added the requires triage New issue that requires categorization label Oct 9, 2022
@CaselIT CaselIT added bug Something isn't working and removed requires triage New issue that requires categorization labels Oct 9, 2022
@CaselIT
Copy link
Member

CaselIT commented Oct 9, 2022

Thanks for reporting

Sadly this will not be fixed in 1.4. In version 2 it will be fixed

@CaselIT CaselIT added the wontfix This will not be worked on label Oct 9, 2022
@CaselIT CaselIT closed this as completed Oct 9, 2022
@CaselIT
Copy link
Member

CaselIT commented Oct 9, 2022

on second though, we can just lie to the type checker I guess, by making Column inherit Mapped only on the stubs.

Do you have any reservation with that @zzzeek ?

@CaselIT CaselIT reopened this Oct 9, 2022
@CaselIT CaselIT added missing type something is missing from the types and removed wontfix This will not be worked on labels Oct 9, 2022
@mehdigmira
Copy link
Contributor Author

Yes that what I meant. Mapped is used for typing essentially, So it's a bit weird that the recommended way of typing columns is wrong from a typing point of view.

@mehdigmira
Copy link
Contributor Author

I think this is a problem in mypy also, but was only hidden by an import issue (#237).
Once the import issue fixed, mypy reports the same errors.

@mehdigmira
Copy link
Contributor Author

I've dug into this a bit: Making Column inherit from Mapped causes some MRO problems, since Mapped and ColumnClause share some base classes.
This doesn't prevent Pyright from working fine with this, but causes some problems for mypy which just forgets the whole class hierarchy (see python/mypy#11427 (comment)).
For mypy, I guess we can use the customize_mro plugin hook to handle this (python/mypy#4527). What do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working missing type something is missing from the types
Projects
None yet
Development

No branches or pull requests

2 participants