From be4e3e675e7f69a2e728af2aa814524173d7a66c Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Thu, 4 Dec 2025 22:00:32 +0100 Subject: [PATCH] behave more like dataclasses when creating them A change in the mechanics of how Python dataclasses are applied to classes that use :class:`.MappedAsDataclass` or :meth:`.registry.mapped_as_dataclass`, to no longer modify the `__annotations__` collection that's on the class, instead manipulating regular class-bound attributes in order to satisfy the class requirements for the dataclass creation function. This works around an issue that has appeared in Python 3.14.1, provides for a much simpler implementation, and also maintains accurate typing information about the attributes as the dataclass is built. Fixes: #13021 Change-Id: I6ae13db7f647ad04e202667d69f2b1bb385c032d --- doc/build/changelog/unreleased_21/13021.rst | 13 ++ lib/sqlalchemy/orm/decl_base.py | 129 +++++++----------- test/orm/declarative/test_dc_transforms.py | 88 ++++++++++-- .../test_dc_transforms_future_anno_sync.py | 88 ++++++++++-- 4 files changed, 221 insertions(+), 97 deletions(-) create mode 100644 doc/build/changelog/unreleased_21/13021.rst diff --git a/doc/build/changelog/unreleased_21/13021.rst b/doc/build/changelog/unreleased_21/13021.rst new file mode 100644 index 0000000000..a0e39faab4 --- /dev/null +++ b/doc/build/changelog/unreleased_21/13021.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, orm + :tickets: 13021 + + A change in the mechanics of how Python dataclasses are applied to classes + that use :class:`.MappedAsDataclass` or + :meth:`.registry.mapped_as_dataclass`, to no longer modify the + `__annotations__` collection that's on the class, instead manipulating + regular class-bound attributes in order to satisfy the class requirements + for the dataclass creation function. This works around an issue that has + appeared in Python 3.14.1, provides for a much simpler implementation, and + also maintains accurate typing information about the attributes as the + dataclass is built. diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py index 2a6d9bd231..4e8d8d4169 100644 --- a/lib/sqlalchemy/orm/decl_base.py +++ b/lib/sqlalchemy/orm/decl_base.py @@ -569,35 +569,60 @@ class _ClassScanAbstractConfig(_ORMClassConfigurator): code="dcmx", ) - annotations = {} - defaults = {} - for item in field_list: - if len(item) == 2: - name, tp = item - elif len(item) == 3: - name, tp, spec = item - defaults[name] = spec - else: - assert False - annotations[name] = tp + if revert: + # the "revert" case is used only by an unmapped mixin class + # that is nonetheless using Mapped construct and needs to + # itself be a dataclass + revert_dict = { + name: self.cls.__dict__[name] + for name in (item[0] for item in field_list) + if name in self.cls.__dict__ + } + else: + revert_dict = None - revert_dict = {} + anno = util.get_annotations(self.cls) - for k, v in defaults.items(): - if k in self.cls.__dict__: - revert_dict[k] = self.cls.__dict__[k] - setattr(self.cls, k, v) + # ensure the class has attributes for only those keys + # where an annotation is present. + for item in field_list: + name = item[0] + if name not in anno and name in self.cls.__dict__: + delattr(self.cls, name) + elif name in self.cls.__dict__ and len(item) == 3: + setattr(self.cls, name, item[2]) - self._apply_dataclasses_to_any_class( - dataclass_setup_arguments, self.cls, annotations - ) + self._assert_dc_arguments(dataclass_setup_arguments) - if revert: - # used for mixin dataclasses; we have to restore the - # mapped_column(), relationship() etc. to the class so these - # take place for a mapped class scan - for k, v in revert_dict.items(): - setattr(self.cls, k, v) + dataclass_callable = dataclass_setup_arguments["dataclass_callable"] + if dataclass_callable is _NoArg.NO_ARG: + dataclass_callable = dataclasses.dataclass + + try: + dataclass_callable( # type: ignore[call-overload] + self.cls, + **{ # type: ignore[call-overload,unused-ignore] + k: v + for k, v in dataclass_setup_arguments.items() + if v is not _NoArg.NO_ARG + and k not in ("dataclass_callable",) + }, + ) + except (TypeError, ValueError) as ex: + raise exc.InvalidRequestError( + f"Python dataclasses error encountered when creating " + f"dataclass for {self.cls.__name__!r}: " + f"{ex!r}. Please refer to Python dataclasses " + "documentation for additional information.", + code="dcte", + ) from ex + finally: + if revert and revert_dict: + # used for mixin dataclasses; we have to restore the + # mapped_column(), relationship() etc. to the class so these + # take place for a mapped class scan + for k, v in revert_dict.items(): + setattr(self.cls, k, v) def _collect_annotation( self, @@ -671,60 +696,6 @@ class _ClassScanAbstractConfig(_ORMClassConfigurator): ) return ca - @classmethod - def _apply_dataclasses_to_any_class( - cls, - dataclass_setup_arguments: _DataclassArguments, - klass: Type[_O], - use_annotations: Mapping[str, _AnnotationScanType], - ) -> None: - cls._assert_dc_arguments(dataclass_setup_arguments) - - dataclass_callable = dataclass_setup_arguments["dataclass_callable"] - if dataclass_callable is _NoArg.NO_ARG: - dataclass_callable = dataclasses.dataclass - - restored: Optional[Any] - - if use_annotations: - # apply constructed annotations that should look "normal" to a - # dataclasses callable, based on the fields present. This - # means remove the Mapped[] container and ensure all Field - # entries have an annotation - restored = util.get_annotations(klass) - klass.__annotations__ = cast("Dict[str, Any]", use_annotations) - else: - restored = None - - try: - dataclass_callable( # type: ignore[call-overload] - klass, - **{ # type: ignore[call-overload,unused-ignore] - k: v - for k, v in dataclass_setup_arguments.items() - if v is not _NoArg.NO_ARG - and k not in ("dataclass_callable",) - }, - ) - except (TypeError, ValueError) as ex: - raise exc.InvalidRequestError( - f"Python dataclasses error encountered when creating " - f"dataclass for {klass.__name__!r}: " - f"{ex!r}. Please refer to Python dataclasses " - "documentation for additional information.", - code="dcte", - ) from ex - finally: - # restore original annotations outside of the dataclasses - # process; for mixins and __abstract__ superclasses, SQLAlchemy - # Declarative will need to see the Mapped[] container inside the - # annotations in order to map subclasses - if use_annotations: - if restored is None: - del klass.__annotations__ - else: - klass.__annotations__ = restored # type: ignore[assignment] # noqa: E501 - @classmethod def _assert_dc_arguments(cls, arguments: _DataclassArguments) -> None: allowed = { diff --git a/test/orm/declarative/test_dc_transforms.py b/test/orm/declarative/test_dc_transforms.py index 46780e3806..889fba753c 100644 --- a/test/orm/declarative/test_dc_transforms.py +++ b/test/orm/declarative/test_dc_transforms.py @@ -148,6 +148,49 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): ), ) + use_future_mode = False + # anno only: use_future_mode = True + + # new docstrings change as of #12168 (adds DONT_SET as default value) + # and #13021 (maintains Mapped[type] as the type when dataclass is + # created) + if use_future_mode: + eq_regex( + A.__doc__, + r"A\(data: 'Mapped\[str\]', " + r"x: 'Mapped" + r"\[(?:Optional\[int\]|int \| None)\]' = " + r", " + r"bs: \"Mapped" + r"\[List\['B'\]\]\" = " + r"\)", + ) + eq_regex( + B.__doc__, + r"B\(data: 'Mapped\[str\]', " + r"x: 'Mapped" + r"\[(?:Optional\[int\]|int \| None)\]' = " + r"\)", + ) + else: + eq_regex( + A.__doc__, + r"A\(data: sqlalchemy.orm.base.Mapped\[str\], " + r"x: sqlalchemy.orm.base.Mapped" + r"\[(?:typing.Optional\[int\]|int \| None)\] = " + r", " + r"bs: sqlalchemy.orm.base.Mapped" + r"\[typing.List\[ForwardRef\('B'\)\]\] = " + r"\)", + ) + eq_regex( + B.__doc__, + r"B\(data: sqlalchemy.orm.base.Mapped\[str\], " + r"x: sqlalchemy.orm.base.Mapped" + r"\[(?:typing.Optional\[int\]|int \| None)\] = " + r"\)", + ) + a2 = A("10", x=5, bs=[B("data1"), B("data2", x=12)]) eq_( repr(a2), @@ -327,7 +370,10 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_(annotations, {MappedClass: {"id": int, "name": str}}) + eq_( + annotations, + {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, + ) elif dc_type.decorator: reg = registry() @@ -339,7 +385,10 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_(annotations, {MappedClass: {"id": int, "name": str}}) + eq_( + annotations, + {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, + ) elif dc_type.superclass: @@ -355,7 +404,10 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): eq_( annotations, - {Mixin: {"id": int}, MappedClass: {"id": int, "name": str}}, + { + Mixin: {"id": Mapped[int]}, + MappedClass: {"name": Mapped[str]}, + }, ) else: dc_type.fail() @@ -1342,7 +1394,10 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): __tablename__ = "child" c: Mapped[int] = mapped_column(primary_key=True) - eq_(collected_annotations, {Mixin: {"b": int}, Child: {"c": int}}) + eq_( + collected_annotations, + {Mixin: {"b": int}, Child: {"c": Mapped[int]}}, + ) eq_regex(repr(Child(6, 7)), r".*\.Child\(b=6, c=7\)") # TODO: get this test to work with future anno mode as well @@ -1378,7 +1433,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): # dataclasses collection. eq_( collected_annotations, - {Mixin: {"b": int}, Child: {"b": int, "c": int}}, + {Mixin: {"b": Mapped[int]}, Child: {"c": Mapped[int]}}, ) eq_regex(repr(Child(6, 7)), r".*\.Child\(b=6, c=7\)") @@ -1587,7 +1642,13 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) if MappedAsDataclass in Book.__mro__: - expected_annotations[Book] = {"id": int, "polymorphic_type": str} + if dataclass_scope.on_mixin: + expected_annotations[Book] = {"id": Mapped[int]} + else: + expected_annotations[Book] = { + "id": Mapped[int], + "polymorphic_type": Mapped[str], + } class Novel(Book): id: Mapped[int] = mapped_column( @@ -1597,7 +1658,10 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Novel] = {"id": int, "description": Optional[str]} + expected_annotations[Novel] = { + "id": Mapped[int], + "description": Mapped[Optional[str]], + } if test_alternative_callable: eq_(collected_annotations, expected_annotations) @@ -1676,8 +1740,14 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Book] = {"id": int, "polymorphic_type": str} - expected_annotations[Novel] = {"id": int, "description": Optional[str]} + expected_annotations[Book] = { + "id": Mapped[int], + "polymorphic_type": Mapped[str], + } + expected_annotations[Novel] = { + "id": Mapped[int], + "description": Mapped[Optional[str]], + } expected_annotations[Mixin] = {} if test_alternative_callable: diff --git a/test/orm/declarative/test_dc_transforms_future_anno_sync.py b/test/orm/declarative/test_dc_transforms_future_anno_sync.py index 7724259396..d0cb0de9c4 100644 --- a/test/orm/declarative/test_dc_transforms_future_anno_sync.py +++ b/test/orm/declarative/test_dc_transforms_future_anno_sync.py @@ -157,6 +157,49 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): ), ) + use_future_mode = False + use_future_mode = True + + # new docstrings change as of #12168 (adds DONT_SET as default value) + # and #13021 (maintains Mapped[type] as the type when dataclass is + # created) + if use_future_mode: + eq_regex( + A.__doc__, + r"A\(data: 'Mapped\[str\]', " + r"x: 'Mapped" + r"\[(?:Optional\[int\]|int \| None)\]' = " + r", " + r"bs: \"Mapped" + r"\[List\['B'\]\]\" = " + r"\)", + ) + eq_regex( + B.__doc__, + r"B\(data: 'Mapped\[str\]', " + r"x: 'Mapped" + r"\[(?:Optional\[int\]|int \| None)\]' = " + r"\)", + ) + else: + eq_regex( + A.__doc__, + r"A\(data: sqlalchemy.orm.base.Mapped\[str\], " + r"x: sqlalchemy.orm.base.Mapped" + r"\[(?:typing.Optional\[int\]|int \| None)\] = " + r", " + r"bs: sqlalchemy.orm.base.Mapped" + r"\[typing.List\[ForwardRef\('B'\)\]\] = " + r"\)", + ) + eq_regex( + B.__doc__, + r"B\(data: sqlalchemy.orm.base.Mapped\[str\], " + r"x: sqlalchemy.orm.base.Mapped" + r"\[(?:typing.Optional\[int\]|int \| None)\] = " + r"\)", + ) + a2 = A("10", x=5, bs=[B("data1"), B("data2", x=12)]) eq_( repr(a2), @@ -340,7 +383,10 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_(annotations, {MappedClass: {"id": int, "name": str}}) + eq_( + annotations, + {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, + ) elif dc_type.decorator: reg = registry() @@ -352,7 +398,10 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_(annotations, {MappedClass: {"id": int, "name": str}}) + eq_( + annotations, + {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, + ) elif dc_type.superclass: @@ -368,7 +417,10 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): eq_( annotations, - {Mixin: {"id": int}, MappedClass: {"id": int, "name": str}}, + { + Mixin: {"id": Mapped[int]}, + MappedClass: {"name": Mapped[str]}, + }, ) else: dc_type.fail() @@ -1357,7 +1409,10 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): __tablename__ = "child" c: Mapped[int] = mapped_column(primary_key=True) - eq_(collected_annotations, {Mixin: {"b": int}, Child: {"c": int}}) + eq_( + collected_annotations, + {Mixin: {"b": int}, Child: {"c": Mapped[int]}}, + ) eq_regex(repr(Child(6, 7)), r".*\.Child\(b=6, c=7\)") # TODO: get this test to work with future anno mode as well @@ -1395,7 +1450,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): # dataclasses collection. eq_( collected_annotations, - {Mixin: {"b": int}, Child: {"b": int, "c": int}}, + {Mixin: {"b": Mapped[int]}, Child: {"c": Mapped[int]}}, ) eq_regex(repr(Child(6, 7)), r".*\.Child\(b=6, c=7\)") @@ -1604,7 +1659,13 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) if MappedAsDataclass in Book.__mro__: - expected_annotations[Book] = {"id": int, "polymorphic_type": str} + if dataclass_scope.on_mixin: + expected_annotations[Book] = {"id": Mapped[int]} + else: + expected_annotations[Book] = { + "id": Mapped[int], + "polymorphic_type": Mapped[str], + } class Novel(Book): id: Mapped[int] = mapped_column( @@ -1614,7 +1675,10 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Novel] = {"id": int, "description": Optional[str]} + expected_annotations[Novel] = { + "id": Mapped[int], + "description": Mapped[Optional[str]], + } if test_alternative_callable: eq_(collected_annotations, expected_annotations) @@ -1693,8 +1757,14 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Book] = {"id": int, "polymorphic_type": str} - expected_annotations[Novel] = {"id": int, "description": Optional[str]} + expected_annotations[Book] = { + "id": Mapped[int], + "polymorphic_type": Mapped[str], + } + expected_annotations[Novel] = { + "id": Mapped[int], + "description": Mapped[Optional[str]], + } expected_annotations[Mixin] = {} if test_alternative_callable: -- 2.47.3