Replace cached_property on slotted classes with a new descriptor instead of writing __getattr__
#1488
+216
−55
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR reimplements
cached_propertyin slotted classes as a slot wrapping descriptor replacing the current generated__getattr__implementation.Accessing the attributes will probably be slightly slower due to going through this descriptor but if the performance is reasonable this will close #1288 #1325 and #1333.
This idea came up again as part of discussion on https://discuss.python.org/t/extensible-cached-property/105188/25. I hadn't realised it would work, but it looks like there was a partial attempt to implement it for attrs before in #1357
I've implemented it for my own attrs/dataclasses-like here (the _SlottedCachedProperty code is mostly identical).
As I saw there were outstanding issues for it and the code is fairly similar I thought you might also find it useful. (Passing all of your existing tests also gave me some confidence that it does work correctly).
Changes:
_SlottedCachedPropertydescriptor class that replaces the slot descriptors after attrs has rebuilt the class.cached_propertyitself, other than reading/writing/deleting a wrapped slot rather than dict entry.__getattr__function you were making previously.__getattr__behaviour that are now just checking__getattr__works in Python.Outstanding issues:
This was already the case for the
__getattr__implementation but I'm now aware of one difference in behaviour this hasn't yet resolved as it would require changing the logic of when you create a new cached property. There's currently a test marked as xfail for this.This is because the cached_property replacement check
attrsdoes skips field names that already exist, sonameis skipped in the check and doesn't get replaced. The descriptor would behave like this, but it's never placed.Pull Request Check List
mainbranch – use a separate branch!Our CI fails if coverage is not 100%.
.pyi).typing-examples/baseline.pyor, if necessary,typing-examples/mypy.py.attr/__init__.pyi, they've also been re-imported inattrs/__init__.pyi.docs/api.rstby hand.@attr.s()and@attrs.define()have to be added by hand too.versionadded,versionchanged, ordeprecateddirectives.The next version is the second number in the current release + 1.
The first number represents the current year.
So if the current version on PyPI is 22.2.0, the next version is gonna be 22.3.0.
If the next version is the first in the new year, it'll be 23.1.0.
attrs.define()andattr.s(), you have to add version directives to both..rstand.mdfiles is written using semantic newlines.changelog.d.