Merge pull request #83 from DavidCEllis/cached-property-preserve-dict

Use a __dict__ for cached_property values if it exists

Author: DavidCEllis

src/ducktools/classbuilder/__init__.py

@@ -877,27 +877,29 @@ def __new__(
                 if k not in {"__weakref__", "__dict__"}:
                     fields[k] = v
 
-            # Special case cached_property
-            # if a cached property is used we need to remove it so that
-            # its attribute can be replaced by a slot, after the class
-            # is constructed wrap the slot with a new special _SlottedCachedProperty
-            # that will store the resulting value in the slot instead of in a dict.
+            # Special case cached_property if there is no `__dict__` attribute
+            # In the case where there is a __dict__ it is not necessary to replace
+            # the cached_property attribute as the dict can be used, otherwise
+            # it needs to be replaced in order to store the value in the slot
+            # created.
+            # Dict access is faster if there is a __dict__ available.
             cached_properties = {}
 
-            # Don't import functools
-            if functools := sys.modules.get("functools"):
-                # Iterate over a copy as we will mutate the original
-                for k, v in ns.copy().items():
-                    if isinstance(v, functools.cached_property):
-                        cached_properties[k] = v
-                        del ns[k]
-                        # Add to slots only if it is not already a slot
-                        slot_attrib = base_attribs.get(k, NOTHING)
-                        if (
-                            slot_attrib is NOTHING
-                            or type(slot_attrib) not in existing_slot_types
-                        ):
-                            slot_values[k] = None
+            if "__dict__" not in slot_values and "__dict__" not in base_attribs:
+                # Don't import functools
+                if functools := sys.modules.get("functools"):
+                    # Iterate over a copy as we will mutate the original
+                    for k, v in ns.copy().items():
+                        if isinstance(v, functools.cached_property):
+                            cached_properties[k] = v
+                            del ns[k]
+                            # Add to slots only if it is not already a slot
+                            slot_attrib = base_attribs.get(k, NOTHING)
+                            if (
+                                slot_attrib is NOTHING
+                                or type(slot_attrib) not in existing_slot_types
+                            ):
+                                slot_values[k] = None
 
             # Place slots *after* everything else to be safe
             ns["__slots__"] = slot_values

tests/prefab/test_subclass_implementation.py

@@ -3,7 +3,7 @@
 
 import pytest
 
-from ducktools.classbuilder import get_flags
+from ducktools.classbuilder import _SlottedCachedProperty  # type: ignore
 from ducktools.classbuilder.prefab import Prefab, Attribute, SlotFields, get_attributes
 
 
@@ -257,7 +257,33 @@ def h2g2(self):
         ex = Example()
         assert not hasattr(ex, "__dict__")
         assert ex.h2g2 == 42
+        assert isinstance(Example.h2g2, _SlottedCachedProperty)
 
+    def test_dict_present_leaves_cached_property(self):
+        # Test cached_property is left in place if there is a __dict__ attribute available
+        class Example(Prefab):
+            __dict__: dict
+            @functools.cached_property
+            def h2g2(self):
+                return 42
+
+        ex = Example()
+        assert ex.h2g2 == 42
+        assert isinstance(Example.h2g2, functools.cached_property)
+
+    def test_dict_inherited_leaves_cached_property(self):
+        # Test it is left in place if __dict__ is inherited
+        class Example(Prefab):
+            __dict__: dict
+
+        class ExampleInherit(Example):
+            @functools.cached_property
+            def h2g2(self):
+                return 42
+
+        ex = ExampleInherit()
+        assert ex.h2g2 == 42
+        assert isinstance(ExampleInherit.h2g2, functools.cached_property)
 
     def test_subclass_cached_property(self):
         # Test we don't suffer from https://github.com/python-attrs/attrs/issues/1333
@@ -293,7 +319,7 @@ class Parent(Prefab):
         # This test exists to document the weirdness
         # Thankfully mypy flags this as an error
         class Child(Parent):
-            @functools.cached_property
+            @functools.cached_property  # type: ignore
             def name(self):
                 return "Bill"
 

tests/test_slotmakermeta.py

@@ -85,9 +85,6 @@ class ExampleBase(metaclass=SlotMakerMeta):
 
 
 class TestCachedProperty:
-    # Test that a cached property causes __dict__ to be
-    # automatically added to __slots__
-
     @graalpy_fails
     def test_no_cached_property(self):
         class Example(metaclass=SlotMakerMeta):
@@ -116,4 +113,4 @@ class Example(metaclass=SlotMakerMeta):
             def cache(self):
                 return 42
 
-        assert Example.__slots__ == {"x": None, "cache": None, "__dict__": "dict for cached property"}
+        assert Example.__slots__ == {"x": None, "__dict__": "dict for cached property"}