diff --git a/changelog.d/cache_slot.change.md b/changelog.d/cache_slot.change.md new file mode 100644 index 000000000..12ca1b74a --- /dev/null +++ b/changelog.d/cache_slot.change.md @@ -0,0 +1 @@ +Fixed docstrings for @cached_property of slotted classes diff --git a/src/attr/_make.py b/src/attr/_make.py index 32e42976e..eb8f20471 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -496,56 +496,73 @@ 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)", - ] - ) +_cached_property_results = {} - lines.extend( - [ - " return __getattr__", - "__getattr__ = wrapper(_cls)", - ] - ) - unique_filename = _generate_unique_filename(cls, "getattr") +def _make_cached_property_uncached(original_cached_property_func, cls): + """Make an ordinary :deco:`property` to replace a :deco:`cached_property` + + The :deco:`property` will work on slotted instances. We'll make a slot + to keep the result of :func:`original_cached_property_func`, named like it, + with the suffix ``_cached``. + """ + name = original_cached_property_func.__name__ + doc = original_cached_property_func.__doc__ + doc_lines = [] + if doc is not None: + doc_lines = doc.splitlines() + if len(doc_lines) == 1: + doc_lines = [' """' + doc_lines[0] + '"""'] + else: + line0 = ' """' + doc_lines[0].strip() + for i, raw_line in enumerate(doc_lines[1:], start=1): + line = raw_line.strip() + if line: + doc_lines[i] = " " + line + else: + doc_lines[i] = "" + if len(doc_lines) > 2 and doc_lines[-2] == doc_lines[-1] == "": + doc_lines = [line0, *doc_lines[1:-1], ' """'] + else: + doc_lines = [line0, *doc_lines[1:], ' """'] + + annotation = inspect.signature( + original_cached_property_func + ).return_annotation + if annotation is inspect.Parameter.empty: + defline = f"def {name}(self):" + elif isinstance( + annotation, + (type, types.FunctionType, types.BuiltinFunctionType), + ): + if annotation.__module__ == "builtins": + defline = f"def {name}(self) -> {annotation.__qualname__}:" + else: + defline = f"def {name}(self) -> {annotation.__module__}.{annotation.__qualname__}:" + else: + defline = f"def {name}(self) -> {annotation}:" + lines = [ + "@property", + defline, + *doc_lines, + " cls = self.__class__", + f" result = cached_property_results.get((cls, '{name}', id(self)), NOTHING)", + " if result is NOTHING:", + f" result = cached_property_results[cls, '{name}', id(self)] = original_cached_property(self)", + " return result", + ] + unique_filename = _generate_unique_filename( + cls, original_cached_property_func + ) glob = { - "cached_properties": cached_properties, - "_cached_setattr_get": _OBJ_SETATTR.__get__, - "original_getattr": original_getattr, + "original_cached_property": original_cached_property_func, + "cached_property_results": _cached_property_results, + "NOTHING": NOTHING, } - - return _linecache_and_compile( - "\n".join(lines), unique_filename, glob, locals={"_cls": cls} - )["__getattr__"] + return _linecache_and_compile("\n".join(lines), unique_filename, glob)[ + name + ] def _frozen_setattrs(self, name, value): @@ -914,22 +931,13 @@ def _create_slots_class(self): 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 - ) + cd[name] = _make_cached_property_uncached(func, self._cls) # We only add the names of attributes that aren't inherited. # Setting __slots__ to inherited attributes wastes memory. diff --git a/tests/test_slots.py b/tests/test_slots.py index a74c32b03..f61f4c3f4 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -5,6 +5,7 @@ """ import functools +import inspect import pickle import weakref @@ -752,6 +753,88 @@ def f(self): assert A(11).f == 11 +def test_slots_cached_property_return_annotation(): + """ + cached_property in slotted class preserves return annotation + """ + + @attr.s(slots=True) + class A: + @functools.cached_property + def f(self) -> int: + return 33 + + return_annotation = inspect.signature(A.f.fget).return_annotation + if isinstance(return_annotation, str): + return_annotation = eval(return_annotation) + assert return_annotation is int + + @attr.s(slots=True) + class B: + @functools.cached_property + def f(self) -> functools.partial: + return functools.partial(print, "Hello, world!") + + return_annotation = inspect.signature(B.f.fget).return_annotation + if isinstance(return_annotation, str): + return_annotation = eval(return_annotation) + assert return_annotation is functools.partial + + @attr.s(slots=True) + class C: + @functools.cached_property + def f(self) -> "attr.NOTHING": + return "wow" + + return_annotation = inspect.signature(C.f.fget).return_annotation + assert return_annotation == "attr.NOTHING" + + +def test_slots_cached_property_has_docstring(): + """ + cached_property in slotted class carries its original docstring + """ + + @attr.s(slots=True) + class A: + x = attr.ib() + + @functools.cached_property + def f(self): + """What an informative docstring!""" + return self.x + + assert A.f.__doc__ == "What an informative docstring!" + + +def test_slots_cached_property_has_multiline_docstring(): + """ + cached_property in slotted class carries the original, multiline docstring + """ + + @attr.s(slots=True) + class A: + x = attr.ib() + + @functools.cached_property + def f(self): + """This function is so well documented, + + I had to put newlines in + + """ + return self.x + + docstring_lines = A.f.__doc__.splitlines() + docstring_lines_dedented = [line.lstrip() for line in docstring_lines] + assert docstring_lines_dedented[:4] == [ + "This function is so well documented,", + "", + "I had to put newlines in", + "", + ] + + def test_slots_cached_property_class_does_not_have__dict__(): """ slotted class with cached property has no __dict__ attribute. @@ -765,7 +848,7 @@ class A: def f(self): return self.x - assert set(A.__slots__) == {"x", "f", "__weakref__"} + assert set(A.__slots__) == {"x", "__weakref__"} assert "__dict__" not in dir(A) @@ -1055,7 +1138,7 @@ class B(A): f: int = attr.ib() assert B(1, 2).f == 2 - assert B.__slots__ == () + assert B.__slots__ == ("f",) def test_slots_cached_property_is_not_called_at_construction():