Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for cached_properties to slotted attrs classes. #1200

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1200.change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Slotted classes now transform `functools.cached_property` decorated methods to support equivalent semantics.
20 changes: 20 additions & 0 deletions docs/how-does-it-work.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,23 @@ Pick what's more important to you.
You should avoid instantiating lots of frozen slotted classes (i.e. `@frozen`) in performance-critical code.

Frozen dict classes have barely a performance impact, unfrozen slotted classes are even *faster* than unfrozen dict classes (i.e. regular classes).


(how-slotted-cached_property)=

## Cached Properties on Slotted Classes.

By default, the standard library `functools.cached_property` decorator does not work on slotted classes,
because it requires a `__dict__` to store the cached value.
This could be surprising when uses *attrs*, as makes using slotted classes so easy,
so attrs will convert `functools.cached_property` decorated methods, when constructing slotted classes.

Getting this working is achieved by:
* Adding names to `__slots__` for the wrapped methods.
* Adding a `__getattr__` method to set values on the wrapped methods.

For most users this should mean that it works transparently.

Note that the implementation does not guarantee that the wrapped method is called
only once in multi-threaded usage. This matches the implementation of `cached_property`
in python v3.12.
1 change: 1 addition & 0 deletions src/attr/_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@


PYPY = platform.python_implementation() == "PyPy"
PY_3_8_PLUS = sys.version_info[:2] >= (3, 8)
PY_3_9_PLUS = sys.version_info[:2] >= (3, 9)
PY310 = sys.version_info[:2] >= (3, 10)
PY_3_12_PLUS = sys.version_info[:2] >= (3, 12)
Expand Down
102 changes: 100 additions & 2 deletions src/attr/_make.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import contextlib
import copy
import enum
import functools
import inspect
import itertools
import linecache
import sys
import types
Expand All @@ -16,6 +18,7 @@
from . import _compat, _config, setters
from ._compat import (
PY310,
PY_3_8_PLUS,
_AnnotationExtractor,
get_generic_base,
)
Expand Down Expand Up @@ -597,6 +600,62 @@ 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():",
" __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(
[
" if hasattr(super(), '__getattr__'):",
" 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()",
]
)

unique_filename = _generate_unique_filename(cls, "getattr")

glob = {
"cached_properties": cached_properties,
"_cached_setattr_get": _obj_setattr.__get__,
"_cls": cls,
"original_getattr": original_getattr,
}

return _make_method(
"__getattr__",
"\n".join(lines),
unique_filename,
glob,
)


def _frozen_setattrs(self, name, value):
"""
Attached to frozen classes as __setattr__.
Expand Down Expand Up @@ -857,9 +916,46 @@ def _create_slots_class(self):
):
names += ("__weakref__",)

if PY_3_8_PLUS:
cached_properties = {
name: cached_property.func
for name, cached_property in cd.items()
if isinstance(cached_property, functools.cached_property)
}
else:
# `functools.cached_property` was introduced in 3.8.
# So can't be used before this.
cached_properties = {}

# Collect methods with a `__class__` reference that are shadowed in the new class.
# To know to update them.
additional_closure_functions_to_update = []
if cached_properties:
# Add cached properties to names for slotting.
names += tuple(cached_properties.keys())
diabolo-dan marked this conversation as resolved.
Show resolved Hide resolved

for name in cached_properties:
# Clear out function from class to avoid clashing.
del cd[name]

class_annotations = _get_annotations(self._cls)
for name, func in cached_properties.items():
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.
slot_names = [name for name in names if name not in base_names]

# There are slots for attributes from current class
# that are defined in parent classes.
# As their descriptors may be overridden by a child class,
Expand All @@ -873,6 +969,7 @@ def _create_slots_class(self):
cd.update(reused_slots)
if self._cache_hash:
slot_names.append(_hash_cache_field)

cd["__slots__"] = tuple(slot_names)

cd["__qualname__"] = self._cls.__qualname__
Expand All @@ -886,7 +983,9 @@ def _create_slots_class(self):
# compiler will bake a reference to the class in the method itself
# as `method.__closure__`. Since we replace the class with a
# clone, we rewrite these references so it keeps working.
for item in cls.__dict__.values():
for item in itertools.chain(
cls.__dict__.values(), additional_closure_functions_to_update
):
if isinstance(item, (classmethod, staticmethod)):
# Class- and staticmethods hide their functions inside.
# These might need to be rewritten as well.
Expand All @@ -909,7 +1008,6 @@ def _create_slots_class(self):
else:
if match:
cell.cell_contents = cls

return cls

def add_repr(self, ns):
Expand Down
26 changes: 24 additions & 2 deletions tests/strategies.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""
Testing strategies for Hypothesis-based tests.
"""

import functools
import keyword
import string

Expand All @@ -13,6 +13,8 @@

import attr

from attr._compat import PY_3_8_PLUS

from .utils import make_class


Expand Down Expand Up @@ -111,13 +113,19 @@ def simple_attrs_with_metadata(draw):

simple_attrs = simple_attrs_without_metadata | simple_attrs_with_metadata()


# Python functions support up to 255 arguments.
list_of_attrs = st.lists(simple_attrs, max_size=3)


@st.composite
def simple_classes(
draw, slots=None, frozen=None, weakref_slot=None, private_attrs=None
draw,
slots=None,
frozen=None,
weakref_slot=None,
private_attrs=None,
cached_property=None,
):
"""
A strategy that generates classes with default non-attr attributes.
Expand Down Expand Up @@ -157,6 +165,7 @@ class HypClass:
pre_init_flag = draw(st.booleans())
post_init_flag = draw(st.booleans())
init_flag = draw(st.booleans())
cached_property_flag = draw(st.booleans())

if pre_init_flag:

Expand All @@ -179,9 +188,22 @@ def init(self, *args, **kwargs):

cls_dict["__init__"] = init

bases = (object,)
if cached_property or (
PY_3_8_PLUS and cached_property is None and cached_property_flag
):

class BaseWithCachedProperty:
@functools.cached_property
def _cached_property(self) -> int:
return 1

bases = (BaseWithCachedProperty,)

return make_class(
"HypClass",
cls_dict,
bases=bases,
slots=slots_flag if slots is None else slots,
frozen=frozen_flag if frozen is None else frozen,
weakref_slot=weakref_flag if weakref_slot is None else weakref_slot,
Expand Down
2 changes: 1 addition & 1 deletion tests/test_3rd_party.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class TestCloudpickleCompat:
Tests for compatibility with ``cloudpickle``.
"""

@given(simple_classes())
@given(simple_classes(cached_property=False))
def test_repr(self, cls):
"""
attrs instances can be pickled and un-pickled with cloudpickle.
Expand Down
Loading