From 2adaae777f70bc90b6e28c177545d3638ce215f9 Mon Sep 17 00:00:00 2001 From: Danny Cooper Date: Thu, 10 Oct 2024 17:11:45 +0100 Subject: [PATCH 1/2] WIP: cached properties implementation using descriptor rather than getattr --- src/attr/_make.py | 101 +++++++++++++++++--------------------------- tests/test_slots.py | 17 ++++++++ 2 files changed, 56 insertions(+), 62 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 274d4121b..818bc97c6 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -56,6 +56,34 @@ _DEFAULT_ON_SETATTR = setters.pipe(setters.convert, setters.validate) +class Desc: + def __init__(self, func, old_desc): + if isinstance(old_desc, Desc): + old_desc = old_desc.old_desc + self.old_desc = old_desc + self.func = func + functools.update_wrapper(self, func) + # self.attrname = func.__name__ + self.__doc__ = func.__doc__ + + def __get__(self, obj, objtype=None): + print(f' getting {obj}, {objtype}') + try: + result = self.old_desc.__get__(obj) + print(f'resulting in {result}') + return result + except TypeError as e: + pass + except AttributeError as e: + print(f'exceptions = {e}') + pass + if obj is None: + return self + print(f'{self.old_desc=}') + result = self.func(obj) + self.old_desc.__set__(obj, result) + return result + class _Nothing(enum.Enum): """ Sentinel to indicate the lack of a value when `None` is ambiguous. @@ -477,62 +505,6 @@ def _transform_attrs( return _Attributes((AttrsClass(attrs), base_attrs, base_attr_map)) -def _make_cached_property_getattr(cached_properties, original_getattr, cls): - lines = [ - # Wrapped to get `__class__` into closure cell for super() - # (It will be replaced with the newly constructed class after construction). - "def wrapper(_cls):", - " __class__ = _cls", - " def __getattr__(self, item, cached_properties=cached_properties, original_getattr=original_getattr, _cached_setattr_get=_cached_setattr_get):", - " func = cached_properties.get(item)", - " if func is not None:", - " result = func(self)", - " _setter = _cached_setattr_get(self)", - " _setter(item, result)", - " return result", - ] - if original_getattr is not None: - lines.append( - " return original_getattr(self, item)", - ) - else: - lines.extend( - [ - " try:", - " return super().__getattribute__(item)", - " except AttributeError:", - " if not hasattr(super(), '__getattr__'):", - " raise", - " return super().__getattr__(item)", - " original_error = f\"'{self.__class__.__name__}' object has no attribute '{item}'\"", - " raise AttributeError(original_error)", - ] - ) - - lines.extend( - [ - " return __getattr__", - "__getattr__ = wrapper(_cls)", - ] - ) - - unique_filename = _generate_unique_filename(cls, "getattr") - - glob = { - "cached_properties": cached_properties, - "_cached_setattr_get": _OBJ_SETATTR.__get__, - "original_getattr": original_getattr, - } - - return _make_method( - "__getattr__", - "\n".join(lines), - unique_filename, - glob, - locals={ - "_cls": cls, - }, - ) def _frozen_setattrs(self, name, value): @@ -767,6 +739,7 @@ def _create_slots_class(self): # Traverse the MRO to collect existing slots # and check for an existing __weakref__. existing_slots = {} + existing_cached_property = [] weakref_inherited = False for base_cls in self._cls.__mro__[1:-1]: if base_cls.__dict__.get("__weakref__", None) is not None: @@ -777,6 +750,9 @@ def _create_slots_class(self): for name in getattr(base_cls, "__slots__", []) } ) + existing_cached_property.update( + getattr(base_cls, '__attrs_cached_properties__', []) + ) base_names = set(self._base_names) @@ -795,28 +771,27 @@ def _create_slots_class(self): if isinstance(cached_property, functools.cached_property) } + cd['__attrs_cached_properties__'] = list(cached_properties.keys()) + # Collect methods with a `__class__` reference that are shadowed in the new class. # To know to update them. + property_calls = {} additional_closure_functions_to_update = [] if cached_properties: + ... class_annotations = _get_annotations(self._cls) for name, func in cached_properties.items(): # Add cached properties to names for slotting. names += (name,) # Clear out function from class to avoid clashing. + del cd[name] additional_closure_functions_to_update.append(func) annotation = inspect.signature(func).return_annotation if annotation is not inspect.Parameter.empty: class_annotations[name] = annotation - original_getattr = cd.get("__getattr__") - if original_getattr is not None: - additional_closure_functions_to_update.append(original_getattr) - cd["__getattr__"] = _make_cached_property_getattr( - cached_properties, original_getattr, self._cls - ) # We only add the names of attributes that aren't inherited. # Setting __slots__ to inherited attributes wastes memory. @@ -842,6 +817,8 @@ def _create_slots_class(self): # Create new class based on old class and our methods. cls = type(self._cls)(self._cls.__name__, self._cls.__bases__, cd) + for name, func in cached_properties.items(): + setattr(cls, name, Desc(func, getattr(cls, name))) # The following is a fix for # . diff --git a/tests/test_slots.py b/tests/test_slots.py index 9af18e5ee..7ee2d8d2f 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -1126,6 +1126,23 @@ def f_2(self): assert obj.f_1 == 1 assert obj.f_2 == 2 +def test_slots_cached_property_retains_doc(): + """ + Cached property's docstring is retained. + """ + + @attr.s(slots=True) + class A: + x = attr.ib() + + @functools.cached_property + def f(self): + """ + This is a docstring. + """ + return self.x + + assert "This is a docstring." in A.f.__doc__ @attr.s(slots=True) class A: From a1a8bf34112887115bb90525a3fe21891b9168b3 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 10 Oct 2024 16:13:57 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/attr/_make.py | 23 +++++++++-------------- tests/test_slots.py | 2 ++ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 818bc97c6..7132a7297 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -67,23 +67,23 @@ def __init__(self, func, old_desc): self.__doc__ = func.__doc__ def __get__(self, obj, objtype=None): - print(f' getting {obj}, {objtype}') + print(f" getting {obj}, {objtype}") try: result = self.old_desc.__get__(obj) - print(f'resulting in {result}') - return result - except TypeError as e: + print(f"resulting in {result}") + return result + except TypeError: pass except AttributeError as e: - print(f'exceptions = {e}') - pass + print(f"exceptions = {e}") if obj is None: return self - print(f'{self.old_desc=}') + print(f"{self.old_desc=}") result = self.func(obj) self.old_desc.__set__(obj, result) return result + class _Nothing(enum.Enum): """ Sentinel to indicate the lack of a value when `None` is ambiguous. @@ -505,8 +505,6 @@ def _transform_attrs( return _Attributes((AttrsClass(attrs), base_attrs, base_attr_map)) - - def _frozen_setattrs(self, name, value): """ Attached to frozen classes as __setattr__. @@ -751,7 +749,7 @@ def _create_slots_class(self): } ) existing_cached_property.update( - getattr(base_cls, '__attrs_cached_properties__', []) + getattr(base_cls, "__attrs_cached_properties__", []) ) base_names = set(self._base_names) @@ -771,14 +769,13 @@ def _create_slots_class(self): if isinstance(cached_property, functools.cached_property) } - cd['__attrs_cached_properties__'] = list(cached_properties.keys()) + cd["__attrs_cached_properties__"] = list(cached_properties.keys()) # Collect methods with a `__class__` reference that are shadowed in the new class. # To know to update them. property_calls = {} additional_closure_functions_to_update = [] if cached_properties: - ... class_annotations = _get_annotations(self._cls) for name, func in cached_properties.items(): # Add cached properties to names for slotting. @@ -791,8 +788,6 @@ def _create_slots_class(self): if annotation is not inspect.Parameter.empty: class_annotations[name] = annotation - - # We only add the names of attributes that aren't inherited. # Setting __slots__ to inherited attributes wastes memory. slot_names = [name for name in names if name not in base_names] diff --git a/tests/test_slots.py b/tests/test_slots.py index 7ee2d8d2f..ab15db610 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -1126,6 +1126,7 @@ def f_2(self): assert obj.f_1 == 1 assert obj.f_2 == 2 + def test_slots_cached_property_retains_doc(): """ Cached property's docstring is retained. @@ -1144,6 +1145,7 @@ def f(self): assert "This is a docstring." in A.f.__doc__ + @attr.s(slots=True) class A: x = attr.ib()