Repository navigation
cached_property no longer works as a data descriptor in Python 3.12 #106292
Description
Bug report
In Python 3.11 and below, when cached_property was inherited from, the __get__ method would always check for the attribute in the cache.
In Python 3.12.0b3 (since #101890 it seems) the __get__ method no longer checks the cache.
This isn't an issue for typical use, but it might be an issue for subclasses.
For example here's a version of cached_property that inherits from the functools version but also allows for a setter (just as @property does).
Since this child class adds a __set__ method, the __get__ method in functools.cached_property will be called before the __dict__ attribute is accessed.
"""Demonstration of cached_property difference in Python 3.12.0b3."""
import functools
class settable_cached_property(functools.cached_property):
def __init__(self, func):
super().__init__(func)
self._setter = self._deleter = None
def setter(self, setter):
self._setter = setter
return self
def __set__(self, obj, value):
if self._setter:
self._setter(obj, value)
obj.__dict__.pop(self.attrname, None)
else:
obj.__dict__[self.attrname] = value
class Thing:
@settable_cached_property
def x(self):
return self.y
thing = Thing()
thing.y = 4
print(f"{thing.x = } (should be 4)")
thing.y = 5
print(f"{thing.x = } (should still be 4)")This new behavior may be intended, but I wanted to make a note of it because it does break a previous (undocumented I believe?) assumption that cached_property could be inherited from and turned into a data descriptor.
Your environment
Python 3.12.0b3 on Ubuntu Linux
Activity
AlexWaygood commentedon Jun 30, 2023
Cc. @carljm
carljm commentedon Jun 30, 2023
Thanks @treyhunner for the report! Yeah, this was unintended and undocumented behavior pre-3.12, that was an accidental side effect of the locking.
We could restore this behavior, but it penalizes performance (double-checks the dict unnecessarily) in the common case (use of
cached_propertydirectly), in order to support an unintended use (subclass that adds a setter). A settable cached property subclass can override__get__to checkobj.__dict__unconditionally (and even still defer tosuper().__get__in case the value is missing.)I would prefer to document this behavior change in What's New. I can do a code search for subclasses of
functools.cached_propertyin OSS libraries; if it seems like this is a common pattern, then maybe we do want to restore the behavior for compatibility, even if it's not the ideal behavior.treyhunner commentedon Jun 30, 2023
@carljm given that this feature was unintended, documenting the change in What's New sounds reasonable to me. 👍
That code example is based on a Python exercise where I discuss how
cached_propertycould be extended. I would guess/hope that there may not be much OSS code floating around that assumescached_propertycould be turned into a data descriptor trivially.carljm commentedon Jul 3, 2023
I went through all 47 hits on sourcegraph code search for regex
class .+\((functools\.)?cached_property\). 46 of them were either inheriting a differentcached_propertyentirely (usually the Django or Werkzeug one), or the subclass did not implement__set__. There was one problematic case, in Dask: https://sourcegraph.com/github.com/dask/dask/-/blob/dask/utils.pyI can submit a PR to Dask before the release of 3.12 to fix this case, but this does suggest that it's possible other people are also doing this, in codebases we don't have access to.
I've realized several things that are swaying my opinion here:
__set__) this case will remain a normal attribute access, thanks tocached_propertybeing a non-data descriptor.cached_propertyand add a__set__method, with the current implementation. There's some error-checking that happens in__get__before you can even safely get the instance dictionary, and this all would have to be copied into the subclass'__get__override.__set__method. It's a bit of a footgun to makecached_propertyno longer cached at all if you just subclass it and add__set__. And I did find enough cases of people doing this (including people doing it incorrectly with Django'scached_property, which also only works as a non-data descriptor) to make me think people will in fact shoot their feet with this footgun.So I am now thinking it may be the safest option to just re-add the extra dict check in
__get__. Does anyone feel strongly that this would not be a good idea?pythongh-106292: restore checking __dict__ in cached_property.__get__
gh-106292: restore checking __dict__ in cached_property.__get__ (#106380
pythongh-106292: restore checking __dict__ in cached_property.__get__ (…
[3.12] gh-106292: restore checking __dict__ in cached_property.__get__ (