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

Wrong dataclass_transform parameter name used for DCTransformDeclarative #9067

Closed
ImogenBits opened this issue Jan 8, 2023 · 3 comments
Closed
Labels
bug Something isn't working dataclasses orm regression something worked and was broken by a change typing pep -484 typing issues. independent of "mypy"
Milestone

Comments

@ImogenBits
Copy link

ImogenBits commented Jan 8, 2023

Describe the bug

This is basically the same issue that happened to pydantic here. Previous versions of PEP 681 had dataclass_transform take a parameter called field_descriptors but this name was changed to field_specifiers. SQLAlchemy still uses the old name for MappedAsDataclass, here.

To Reproduce

from typing import reveal_type
from sqlalchemy.orm import DeclarativeBase, mapped_column, Mapped, MappedAsDataclass

class Model(MappedAsDataclass, DeclarativeBase):
    a: Mapped[int] = mapped_column(default=1)

reveal_type(Model.__init__)

Error

Type of "Model.__init__" is "(self: Model, a: SQLCoreOperations[int] | int = mapped_column(default=1)) -> None"

Versions

  • Python: 3.11
  • SQLAlchemy: 2.0.0rc1

Additional context

The type of the init method should be (self: Model, a: SQLCoreOperations[int] | int = 1) -> None, which it is if the dataclass_transform call on DCTransformDeclarative uses the correct parameter name.

@ImogenBits ImogenBits added the requires triage New issue that requires categorization label Jan 8, 2023
@CaselIT
Copy link
Member

CaselIT commented Jan 8, 2023

Hi,

Thanks for reporting. Since mypy doesn't support this I guess it was not detected, since we are not currently running pyright on the code

@zzzeek maybe it would be good to try have also pyright run on the code (assuming it would detect this error)?

@CaselIT CaselIT added this to the 2.0 final milestone Jan 8, 2023
@CaselIT CaselIT added bug Something isn't working regression something worked and was broken by a change dataclasses typing pep -484 typing issues. independent of "mypy" orm and removed requires triage New issue that requires categorization labels Jan 8, 2023
@CaselIT
Copy link
Member

CaselIT commented Jan 8, 2023

Hi,

Thanks for reporting. Since mypy doesn't support this I guess it was not detected, since we are not currently running pyright on the code

@zzzeek maybe it would be good to try have also pyright run on the code (assuming it would detect this error)?

It seems pyright still accepts both, so it would not have helped in any case:
https://github.com/microsoft/pyright/blob/ddc6932b9c53b09b9b32926addbcefcab37b9b2a/packages/pyright-internal/src/analyzer/dataClasses.ts#L802-L807

@sqla-tester
Copy link
Collaborator

Federico Caselli has proposed a fix for this issue in the main branch:

Use field_specifiers instead of deprecated field_descriptors https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4346

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dataclasses orm regression something worked and was broken by a change typing pep -484 typing issues. independent of "mypy"
Projects
None yet
Development

No branches or pull requests

3 participants