From d7b8b89085d6f6bfe333fa4ff86ed9a54b68d67d Mon Sep 17 00:00:00 2001 From: Arie Bovenberg Date: Fri, 18 Mar 2022 08:09:55 +0100 Subject: [PATCH 1/6] dataclass(slots=True) now takes inherited slots into account --- Doc/library/dataclasses.rst | 7 ++++ Lib/dataclasses.py | 23 +++++++++++- Lib/test/test_dataclasses.py | 35 ++++++++++++++++--- .../2022-03-18-17-25-57.bpo-46382.zQUJ66.rst | 2 ++ 4 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index 0f6985f0ba8c42..f70a2a029e66d6 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -188,6 +188,13 @@ Module contents .. versionadded:: 3.10 + .. versionchanged:: 3.11 + If a field name is already included in the ``__slots__`` + of a base class, it will not be included in the generated ``__slots__``. + Therefore, do not use ``__slots__`` to retrieve the field names of a + dataclass. Use :func:`fields` instead. + + ``field``\s may optionally specify a default value, using normal Python syntax:: diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index b327462080f993..6be7c7b5de917f 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -6,6 +6,7 @@ import keyword import builtins import functools +import itertools import abc import _thread from types import FunctionType, GenericAlias @@ -1122,6 +1123,20 @@ def _dataclass_setstate(self, state): object.__setattr__(self, field.name, value) +def _get_slots(cls): + match cls.__dict__.get('__slots__'): + case None: + return + case str(slot): + yield slot + # Slots may be any iterable, but we cannot handle an iterator + # because it will already be (partially) consumed. + case iterable if not hasattr(iterable, '__next__'): + yield from iterable + case _: + raise TypeError(f"Slots of '{cls.__name__}' cannot be determined") + + def _add_slots(cls, is_frozen): # Need to create a new class, since we can't set __slots__ # after a class has been created. @@ -1133,7 +1148,13 @@ def _add_slots(cls, is_frozen): # Create a new dict for our new class. cls_dict = dict(cls.__dict__) field_names = tuple(f.name for f in fields(cls)) - cls_dict['__slots__'] = field_names + # Make sure slots don't overlap with those in base classes. + inherited_slots = set( + itertools.chain.from_iterable(map(_get_slots, cls.__mro__[1:-1])) + ) + cls_dict["__slots__"] = tuple( + itertools.filterfalse(inherited_slots.__contains__, field_names) + ) for field_name in field_names: # Remove our attributes, if present. They'll still be # available in _MARKER. diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 2f37ecdfca6b4d..e4f42493070d37 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -2926,17 +2926,26 @@ class C: x: int def test_generated_slots_value(self): + + class Root: + __slots__ = {'x'} + + class Root2(Root): + __slots__ = 'aa' + @dataclass(slots=True) - class Base: - x: int + class Base(Root2): + y: int - self.assertEqual(Base.__slots__, ('x',)) + self.assertEqual(Base.__slots__, ('y', )) @dataclass(slots=True) class Delivered(Base): - y: int + aa: float + x: str + z: int - self.assertEqual(Delivered.__slots__, ('x', 'y')) + self.assertEqual(Delivered.__slots__, ('z', )) @dataclass class AnotherDelivered(Base): @@ -2944,6 +2953,22 @@ class AnotherDelivered(Base): self.assertTrue('__slots__' not in AnotherDelivered.__dict__) + def test_cant_inherit_from_iterator_slots(self): + + class Root: + __slots__ = iter(['a']) + + class Root2(Root): + __slots__ = ('b', ) + + with self.assertRaisesRegex( + TypeError, + "^Slots of 'Root' cannot be determined" + ): + @dataclass(slots=True) + class C(Root2): + x: int + def test_returns_new_class(self): class A: x: int diff --git a/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst b/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst new file mode 100644 index 00000000000000..733b615822070e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst @@ -0,0 +1,2 @@ +:class:`~dataclasses.dataclass` ``slots=True`` now accounts for slots in base classes. +Patch by Arie Bovenberg. From 0c50512de7eda8b653131ed9165f2fa4361cebaf Mon Sep 17 00:00:00 2001 From: Arie Bovenberg Date: Fri, 18 Mar 2022 18:47:15 +0100 Subject: [PATCH 2/6] clarify dataclass slots docs, expand tests --- Doc/library/dataclasses.rst | 5 ++++- Lib/test/test_dataclasses.py | 16 +++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Doc/library/dataclasses.rst b/Doc/library/dataclasses.rst index f70a2a029e66d6..08568da19d71ae 100644 --- a/Doc/library/dataclasses.rst +++ b/Doc/library/dataclasses.rst @@ -190,9 +190,12 @@ Module contents .. versionchanged:: 3.11 If a field name is already included in the ``__slots__`` - of a base class, it will not be included in the generated ``__slots__``. + of a base class, it will not be included in the generated ``__slots__`` + to prevent `overriding them `_. Therefore, do not use ``__slots__`` to retrieve the field names of a dataclass. Use :func:`fields` instead. + To be able to determine inherited slots, + base class ``__slots__`` may be any iterable, but *not* an iterator. ``field``\s may optionally specify a default value, using normal diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index e4f42493070d37..f4d1928003c10e 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -2931,27 +2931,33 @@ class Root: __slots__ = {'x'} class Root2(Root): + __slots__ = {'k': '...', 'j': ''} + + class Root3(Root2): + __slots__ = ['h'] + + class Root4(Root3): __slots__ = 'aa' @dataclass(slots=True) - class Base(Root2): + class Base(Root4): y: int self.assertEqual(Base.__slots__, ('y', )) @dataclass(slots=True) - class Delivered(Base): + class Derived(Base): aa: float x: str z: int - self.assertEqual(Delivered.__slots__, ('z', )) + self.assertEqual(Derived.__slots__, ('z', )) @dataclass - class AnotherDelivered(Base): + class AnotherDerived(Base): z: int - self.assertTrue('__slots__' not in AnotherDelivered.__dict__) + self.assertTrue('__slots__' not in AnotherDerived.__dict__) def test_cant_inherit_from_iterator_slots(self): From 179b47ea68da43734c4705eb73771bcfe27880cd Mon Sep 17 00:00:00 2001 From: Arie Bovenberg Date: Sat, 19 Mar 2022 08:44:01 +0100 Subject: [PATCH 3/6] improved checks in dataclass slots test --- Lib/test/test_dataclasses.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index f4d1928003c10e..198c4dc2910d3f 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -2942,6 +2942,8 @@ class Root4(Root3): @dataclass(slots=True) class Base(Root4): y: int + j: str + h: str self.assertEqual(Base.__slots__, ('y', )) @@ -2950,6 +2952,8 @@ class Derived(Base): aa: float x: str z: int + k: str + h: str self.assertEqual(Derived.__slots__, ('z', )) From 5d23ba14193b07099e2348c8b66d319969c8b0cc Mon Sep 17 00:00:00 2001 From: Arie Bovenberg Date: Sat, 19 Mar 2022 11:34:03 +0100 Subject: [PATCH 4/6] use specific unittest assert for membership test Co-authored-by: Alex Waygood --- Lib/test/test_dataclasses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 198c4dc2910d3f..847bcd46a69268 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -2961,7 +2961,7 @@ class Derived(Base): class AnotherDerived(Base): z: int - self.assertTrue('__slots__' not in AnotherDerived.__dict__) + self.assertNotIn('__slots__', AnotherDerived.__dict__) def test_cant_inherit_from_iterator_slots(self): From e11bd828ff02dea767729e0943bb76b9e46b615f Mon Sep 17 00:00:00 2001 From: Arie Bovenberg Date: Sat, 19 Mar 2022 11:39:13 +0100 Subject: [PATCH 5/6] fix ref in news item --- .../next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst b/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst index 733b615822070e..8ec0ccaecddebd 100644 --- a/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst +++ b/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst @@ -1,2 +1,2 @@ -:class:`~dataclasses.dataclass` ``slots=True`` now accounts for slots in base classes. +:func:`~dataclasses.dataclass` ``slots=True`` now accounts for slots in base classes. Patch by Arie Bovenberg. From 9af5809d91a53bd8cfc138ad8d3df32937590940 Mon Sep 17 00:00:00 2001 From: Arie Bovenberg Date: Sat, 19 Mar 2022 15:02:45 +0100 Subject: [PATCH 6/6] tweak news item --- .../next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst b/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst index 8ec0ccaecddebd..9bec94969cb4cf 100644 --- a/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst +++ b/Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst @@ -1,2 +1,2 @@ -:func:`~dataclasses.dataclass` ``slots=True`` now accounts for slots in base classes. -Patch by Arie Bovenberg. +:func:`~dataclasses.dataclass` ``slots=True`` now correctly omits slots already +defined in base classes. Patch by Arie Bovenberg.