functools.cached_property is not supported for setattr

[hongweipeng]

BPO	38524
Nosy	@ncoghlan, @taleinat, @carljm, @stevendaprano, @serhiy-storchaka, @sir-sigurd, @hongweipeng, @miss-islington, @DahlitzFlorian
PRs	bpo-38524: add note about the implicit and explicit calling of __set_name__ #17364 [3.8] bpo-38524: document implicit and explicit calling of descriptors' __set_name__ (GH-17364) #17401 [3.7] bpo-38524: document implicit and explicit calling of descriptors' __set_name__ (GH-17364) #17402 bpo-38524: clarify example a bit and improve formatting #17406 [3.8] bpo-38524: clarify example a bit and improve formatting (GH-17406) #17411 [3.7] bpo-38524: clarify example a bit and improve formatting (GH-17406) #17412

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-11-28.05:33:36.308>
created_at = <Date 2019-10-19.07:30:51.621>
labels = ['easy', '3.7', '3.8', '3.9', 'docs']
title = 'functools.cached_property is not supported for setattr'
updated_at = <Date 2019-11-28.05:33:36.306>
user = 'https://github.com/hongweipeng'

bugs.python.org fields:

activity = <Date 2019-11-28.05:33:36.306>
actor = 'taleinat'
assignee = 'docs@python'
closed = True
closed_date = <Date 2019-11-28.05:33:36.308>
closer = 'taleinat'
components = ['Documentation']
creation = <Date 2019-10-19.07:30:51.621>
creator = 'hongweipeng'
dependencies = []
files = []
hgrepos = []
issue_num = 38524
keywords = ['patch', 'easy']
message_count = 18.0
messages = ['354929', '354930', '354934', '354942', '354943', '355002', '355004', '355006', '356539', '357325', '357334', '357559', '357560', '357561', '357593', '357598', '357599', '357600']
nosy_count = 10.0
nosy_names = ['ncoghlan', 'taleinat', 'carljm', 'steven.daprano', 'docs@python', 'serhiy.storchaka', 'sir-sigurd', 'hongweipeng', 'miss-islington', 'DahlitzFlorian']
pr_nums = ['17364', '17401', '17402', '17406', '17411', '17412']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38524'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[hongweipeng]

from functools import cached_property
def age(self):
    return 10
class A:
    def __init__(self):
        setattr(self.__class__, 'age', property(age))
        setattr(self.__class__, 'age3', cached_property(age))

    age2 = cached_property(age)

a = A()
print(a.age)    # 10
print(a.age2)   # 10
print(a.age3)   # TypeError: Cannot use cached_property instance without calling __set_name__

Is it expected?

[stevendaprano]

The documentation doesn't mention __set_name__, but it does say that cached_property is useful for properties which are "effectively immutable".

The __set_name__ error message is pretty cryptic, that seems to have something to do with this PEP:

https://www.python.org/dev/peps/pep-0487/

but why it is relevant here, I don't know. I would expect that cached_property should apply only to read-only properties.

So I think that at the very least, this needs better documentation and a better error message.

[serhiy-storchaka]

You have to call __set_name__() if you create a cached_property dynamically outside of the class namespace.

p = cached_property(age)
p.__set_name__(cls, 'age3')
cls.age3 = p

(And setattr() is not needed here.)

[hongweipeng]

Is it possible to add an attrname parameter? It is more convenient to use.

class cached_property:
    def __init__(self, func, attrname=None):
        self.attrname = attrname
        ...

cls.age3 = cached_property(age, 'age3')

[serhiy-storchaka]

See bpo-38517.

For now we need to better document the use of __set_name__ with cached_property.

[ncoghlan]

Another interesting question this raises is whether type.setattr should be checking for values that have __set_name__ methods defined and calling those methods automatically.

Unfortunately, I think the answer to that is "If we'd thought of that when writing PEP-487, then sure, but it's too late now, as too many folks will be calling it explicitly, and implicitly calling it a second time will cause other, potentially harder to find problems".

So +1 for updating the cached_property docs specifically to mention this problem

However, I'm also wondering if there's somewhere else we should be mentioning it as a general problem.

Perhaps in the docs for __set_name__ itself, noting it as something that authors of descriptors *using* __set_name__ should mention in their docs, with suggested wording?

Something like:

===
`set_nameis only called implicitly as part of thetype`` constructor, so it will need to be called explicitly with the appropriate parameters when a descriptor is added to a class after initial creation::

    descr = custom_descriptor()
    cls.attr = descr
    descr.__set_name__(cls, 'attr')

===

(The normal sequence is for the descriptor to already be part of the class namespace before __set_name__ gets called)

[ncoghlan]

Regarding the "attrname" property idea: unfortunately, that won't work, as __set_name__ doesn't just provide the attribute name, it also provides a reference to the class itself.

cached_property needs both pieces of information, not just the attribute name.

[ncoghlan]

After writing my previous comment, I double-checked the code, and cached_propery is actually one of the cases where a simple "self.attrname = 'age3'" *will* do the right thing, as cached_property never checks the class information.

That won't work in the general case though, and I think it makes more sense for the cached_property documentation to provide advice that will generalise to arbitrary descriptors (i.e. call descr.__set_name__(target, "attr"))

[taleinat]

I agree with Nick: While possible, this would be unnecessarily confusing.

As Serhiy wrote, the proper way to address the situation seems to be improving the documentation. A PR would be welcome!

[DahlitzFlorian]

It would be an honor for me to work on this issue (updating the docs) as my first CPython contribution.

[taleinat]

Hi Florian! We'd be happy to have you do this, please go ahead.

When you have a PR ready, you're welcome to request my review.