cached_property no longer works as a data descriptor in Python 3.12

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

Linked PRs

@carljm

Cc. @carljm

@treyhunner

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_property directly), in order to support an unintended use (subclass that adds a setter). A settable cached property subclass can override __get__ to check obj.__dict__ unconditionally (and even still defer to super().__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_property in 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.

@carljm

@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_property could be extended. I would guess/hope that there may not be much OSS code floating around that assumes cached_property could be turned into a data descriptor trivially.

I went through all 47 hits on sourcegraph code search for regex class .+\((functools\.)?cached_property\). 46 of them were either inheriting a different cached_property entirely (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.py

I 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:

The performance impact of double-checking the dictionary is only on the first access of the cached property. The performance-sensitive case is repeat access, and (for users who don't add __set__) this case will remain a normal attribute access, thanks to cached_property being a non-data descriptor.
It's more difficult than I realized to correctly subclass cached_property and 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.
Many Python users may not clearly understand the difference between data and non-data descriptors, and therefore may not understand the implications of adding a __set__ method. It's a bit of a footgun to make cached_property no 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's cached_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__ (

Navigation Menu

Repository navigation

cached_property no longer works as a data descriptor in Python 3.12 #106292

Bug report

Your environment

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

cached_property no longer works as a data descriptor in Python 3.12 #106292

Description

Bug report

Your environment

Linked PRs

Activity

AlexWaygood commented on Jun 30, 2023

carljm commented on Jun 30, 2023

treyhunner commented on Jun 30, 2023

carljm commented on Jul 3, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions