Skip to content

Commit

Permalink
Slotted cached property reference (#1221)
Browse files Browse the repository at this point in the history
* Remove undesirable original class reference when using slotted cached_property

* Update closure cells for slotted cached property functions

* Changelog

* fixup py 3.7 test
  • Loading branch information
diabolo-dan authored Jan 4, 2024
1 parent 5b0a4e6 commit 26d8dd7
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 8 deletions.
2 changes: 2 additions & 0 deletions changelog.d/1221.change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Allow original slotted cached_property classes to be cleaned by GC.
Allow super calls in slotted cached properties.
16 changes: 11 additions & 5 deletions src/attr/_make.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,11 @@ def _compile_and_eval(script, globs, locs=None, filename=""):
eval(bytecode, globs, locs)


def _make_method(name, script, filename, globs):
def _make_method(name, script, filename, globs, locals=None):
"""
Create the method with the script given and return the method object.
"""
locs = {}
locs = {} if locals is None else locals

# In order of debuggers like PDB being able to step through the code,
# we add a fake linecache entry.
Expand Down Expand Up @@ -608,7 +608,7 @@ def _make_cached_property_getattr(
lines = [
# Wrapped to get `__class__` into closure cell for super()
# (It will be replaced with the newly constructed class after construction).
"def wrapper():",
"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)",
Expand All @@ -635,7 +635,7 @@ def _make_cached_property_getattr(
lines.extend(
[
" return __getattr__",
"__getattr__ = wrapper()",
"__getattr__ = wrapper(_cls)",
]
)

Expand All @@ -644,7 +644,6 @@ def _make_cached_property_getattr(
glob = {
"cached_properties": cached_properties,
"_cached_setattr_get": _obj_setattr.__get__,
"_cls": cls,
"original_getattr": original_getattr,
}

Expand All @@ -653,6 +652,9 @@ def _make_cached_property_getattr(
"\n".join(lines),
unique_filename,
glob,
locals={
"_cls": cls,
},
)


Expand Down Expand Up @@ -938,6 +940,10 @@ def _create_slots_class(self):
# Clear out function from class to avoid clashing.
del cd[name]

additional_closure_functions_to_update.extend(
cached_properties.values()
)

class_annotations = _get_annotations(self._cls)
for name, func in cached_properties.items():
annotation = inspect.signature(func).return_annotation
Expand Down
25 changes: 22 additions & 3 deletions tests/test_make.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
"""
Tests for `attr._make`.
"""


import copy
import functools
import gc
Expand All @@ -23,7 +21,7 @@
import attr

from attr import _config
from attr._compat import PY310
from attr._compat import PY310, PY_3_8_PLUS
from attr._make import (
Attribute,
Factory,
Expand Down Expand Up @@ -1773,6 +1771,27 @@ class C2(C):

assert [C2] == C.__subclasses__()

@pytest.mark.skipif(not PY_3_8_PLUS, reason="cached_property is 3.8+")
def test_no_references_to_original_when_using_cached_property(self):
"""
When subclassing a slotted class and using cached property, there are no stray references to the original class.
"""

@attr.s(slots=True)
class C:
pass

@attr.s(slots=True)
class C2(C):
@functools.cached_property
def value(self) -> int:
return 0

# The original C2 is in a reference cycle, so force a collect:
gc.collect()

assert [C2] == C.__subclasses__()

def _get_copy_kwargs(include_slots=True):
"""
Generate a list of compatible attr.s arguments for the `copy` tests.
Expand Down
19 changes: 19 additions & 0 deletions tests/test_slots.py
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,25 @@ class AB(A, B):
assert ab.h == "h"


@pytest.mark.skipif(not PY_3_8_PLUS, reason="cached_property is 3.8+")
def test_slotted_cached_property_can_access_super():
"""
Multiple sub-classes shouldn't break cached properties.
"""

@attr.s(slots=True)
class A:
x = attr.ib(kw_only=True)

@attr.s(slots=True)
class B(A):
@functools.cached_property
def f(self):
return super().x * 2

assert B(x=1).f == 2


@pytest.mark.skipif(not PY_3_8_PLUS, reason="cached_property is 3.8+")
def test_slots_sub_class_avoids_duplicated_slots():
"""
Expand Down

0 comments on commit 26d8dd7

Please sign in to comment.