From: Michael Bayer Date: Sat, 6 Dec 2025 15:07:25 +0000 (+0000) Subject: Revert "behave more like dataclasses when creating them" X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4ff1d604f9bec061fb1936b80d3ed09979d930e8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Revert "behave more like dataclasses when creating them" This reverts commit be4e3e675e7f69a2e728af2aa814524173d7a66c. Reason for revert: everything has passed very well on jenkins, however GH actions is showing the two new test_cpython_142214 tests from 133f14dabed44f7398039 suddenly failing across hundreds of scenarios. given the risk of this change since dataclasses are very weird, need to see what this is about. im suspecting point release changes in older pythons like 3.10, 3.11, etc. which seems a bit ominous. Change-Id: I7c98085b5e3482cad3291194e2ab1f8018db2eff --- diff --git a/doc/build/changelog/unreleased_21/13021.rst b/doc/build/changelog/unreleased_21/13021.rst deleted file mode 100644 index a0e39faab4..0000000000 --- a/doc/build/changelog/unreleased_21/13021.rst +++ /dev/null @@ -1,13 +0,0 @@ -.. 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 4e8d8d4169..2a6d9bd231 100644 --- a/lib/sqlalchemy/orm/decl_base.py +++ b/lib/sqlalchemy/orm/decl_base.py @@ -569,60 +569,35 @@ class _ClassScanAbstractConfig(_ORMClassConfigurator): code="dcmx", ) - 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 - - anno = util.get_annotations(self.cls) - - # ensure the class has attributes for only those keys - # where an annotation is present. + annotations = {} + defaults = {} 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]) + if len(item) == 2: + name, tp = item + elif len(item) == 3: + name, tp, spec = item + defaults[name] = spec + else: + assert False + annotations[name] = tp - self._assert_dc_arguments(dataclass_setup_arguments) + revert_dict = {} - dataclass_callable = dataclass_setup_arguments["dataclass_callable"] - if dataclass_callable is _NoArg.NO_ARG: - dataclass_callable = dataclasses.dataclass + for k, v in defaults.items(): + if k in self.cls.__dict__: + revert_dict[k] = self.cls.__dict__[k] + setattr(self.cls, k, v) - 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) + self._apply_dataclasses_to_any_class( + dataclass_setup_arguments, self.cls, annotations + ) + + 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) def _collect_annotation( self, @@ -696,6 +671,60 @@ 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 889fba753c..46780e3806 100644 --- a/test/orm/declarative/test_dc_transforms.py +++ b/test/orm/declarative/test_dc_transforms.py @@ -148,49 +148,6 @@ 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), @@ -370,10 +327,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_( - annotations, - {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, - ) + eq_(annotations, {MappedClass: {"id": int, "name": str}}) elif dc_type.decorator: reg = registry() @@ -385,10 +339,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_( - annotations, - {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, - ) + eq_(annotations, {MappedClass: {"id": int, "name": str}}) elif dc_type.superclass: @@ -404,10 +355,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): eq_( annotations, - { - Mixin: {"id": Mapped[int]}, - MappedClass: {"name": Mapped[str]}, - }, + {Mixin: {"id": int}, MappedClass: {"id": int, "name": str}}, ) else: dc_type.fail() @@ -1394,10 +1342,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): __tablename__ = "child" c: Mapped[int] = mapped_column(primary_key=True) - eq_( - collected_annotations, - {Mixin: {"b": int}, Child: {"c": Mapped[int]}}, - ) + eq_(collected_annotations, {Mixin: {"b": int}, Child: {"c": 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 @@ -1433,7 +1378,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): # dataclasses collection. eq_( collected_annotations, - {Mixin: {"b": Mapped[int]}, Child: {"c": Mapped[int]}}, + {Mixin: {"b": int}, Child: {"b": int, "c": int}}, ) eq_regex(repr(Child(6, 7)), r".*\.Child\(b=6, c=7\)") @@ -1642,13 +1587,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) if MappedAsDataclass in Book.__mro__: - if dataclass_scope.on_mixin: - expected_annotations[Book] = {"id": Mapped[int]} - else: - expected_annotations[Book] = { - "id": Mapped[int], - "polymorphic_type": Mapped[str], - } + expected_annotations[Book] = {"id": int, "polymorphic_type": str} class Novel(Book): id: Mapped[int] = mapped_column( @@ -1658,10 +1597,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Novel] = { - "id": Mapped[int], - "description": Mapped[Optional[str]], - } + expected_annotations[Novel] = {"id": int, "description": Optional[str]} if test_alternative_callable: eq_(collected_annotations, expected_annotations) @@ -1740,14 +1676,8 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Book] = { - "id": Mapped[int], - "polymorphic_type": Mapped[str], - } - expected_annotations[Novel] = { - "id": Mapped[int], - "description": Mapped[Optional[str]], - } + expected_annotations[Book] = {"id": int, "polymorphic_type": str} + expected_annotations[Novel] = {"id": int, "description": 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 d0cb0de9c4..7724259396 100644 --- a/test/orm/declarative/test_dc_transforms_future_anno_sync.py +++ b/test/orm/declarative/test_dc_transforms_future_anno_sync.py @@ -157,49 +157,6 @@ 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), @@ -383,10 +340,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_( - annotations, - {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, - ) + eq_(annotations, {MappedClass: {"id": int, "name": str}}) elif dc_type.decorator: reg = registry() @@ -398,10 +352,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] - eq_( - annotations, - {MappedClass: {"id": Mapped[int], "name": Mapped[str]}}, - ) + eq_(annotations, {MappedClass: {"id": int, "name": str}}) elif dc_type.superclass: @@ -417,10 +368,7 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): eq_( annotations, - { - Mixin: {"id": Mapped[int]}, - MappedClass: {"name": Mapped[str]}, - }, + {Mixin: {"id": int}, MappedClass: {"id": int, "name": str}}, ) else: dc_type.fail() @@ -1409,10 +1357,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): __tablename__ = "child" c: Mapped[int] = mapped_column(primary_key=True) - eq_( - collected_annotations, - {Mixin: {"b": int}, Child: {"c": Mapped[int]}}, - ) + eq_(collected_annotations, {Mixin: {"b": int}, Child: {"c": 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 @@ -1450,7 +1395,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): # dataclasses collection. eq_( collected_annotations, - {Mixin: {"b": Mapped[int]}, Child: {"c": Mapped[int]}}, + {Mixin: {"b": int}, Child: {"b": int, "c": int}}, ) eq_regex(repr(Child(6, 7)), r".*\.Child\(b=6, c=7\)") @@ -1659,13 +1604,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) if MappedAsDataclass in Book.__mro__: - if dataclass_scope.on_mixin: - expected_annotations[Book] = {"id": Mapped[int]} - else: - expected_annotations[Book] = { - "id": Mapped[int], - "polymorphic_type": Mapped[str], - } + expected_annotations[Book] = {"id": int, "polymorphic_type": str} class Novel(Book): id: Mapped[int] = mapped_column( @@ -1675,10 +1614,7 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Novel] = { - "id": Mapped[int], - "description": Mapped[Optional[str]], - } + expected_annotations[Novel] = {"id": int, "description": Optional[str]} if test_alternative_callable: eq_(collected_annotations, expected_annotations) @@ -1757,14 +1693,8 @@ class DataclassesForNonMappedClassesTest(fixtures.TestBase): ) description: Mapped[Optional[str]] - expected_annotations[Book] = { - "id": Mapped[int], - "polymorphic_type": Mapped[str], - } - expected_annotations[Novel] = { - "id": Mapped[int], - "description": Mapped[Optional[str]], - } + expected_annotations[Book] = {"id": int, "polymorphic_type": str} + expected_annotations[Novel] = {"id": int, "description": Optional[str]} expected_annotations[Mixin] = {} if test_alternative_callable: