classification
Title: functools.cached_property is not supported for setattr
Type: Stage: resolved
Components: Documentation Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: DahlitzFlorian, carljm, docs@python, hongweipeng, miss-islington, ncoghlan, serhiy.storchaka, sir-sigurd, steven.daprano, taleinat
Priority: normal Keywords: easy, patch

Created on 2019-10-19 07:30 by hongweipeng, last changed 2019-11-28 05:33 by taleinat. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17364 merged DahlitzFlorian, 2019-11-23 14:21
PR 17401 merged miss-islington, 2019-11-27 08:47
PR 17402 merged miss-islington, 2019-11-27 08:47
PR 17406 merged taleinat, 2019-11-27 13:25
PR 17411 merged miss-islington, 2019-11-28 05:22
PR 17412 merged miss-islington, 2019-11-28 05:22
Messages (18)
msg354929 - (view) Author: hongweipeng (hongweipeng) * Date: 2019-10-19 07:30
```
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?
msg354930 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2019-10-19 07:53
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.
msg354934 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-19 09:23
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.)
msg354942 - (view) Author: hongweipeng (hongweipeng) * Date: 2019-10-19 14:55
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')
msg354943 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-19 14:58
See issue38517.

For now we need to better document the use of __set_name__ with cached_property.
msg355002 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-10-20 13:28
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_name__`` is only called implicitly as part of the ``type`` 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)
msg355004 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-10-20 13:30
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.
msg355006 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-10-20 13:37
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")`)
msg356539 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-13 16:11
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!
msg357325 - (view) Author: Florian Dahlitz (DahlitzFlorian) * Date: 2019-11-22 21:26
It would be an honor for me to work on this issue (updating the docs) as my first CPython contribution.
msg357334 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-22 23:06
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.
msg357559 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-27 08:46
New changeset 1bddf890e595a865414645c6041733043c4081f8 by Tal Einat (Florian Dahlitz) in branch 'master':
bpo-38524: document implicit and explicit calling of descriptors' __set_name__ (GH-17364)
https://github.com/python/cpython/commit/1bddf890e595a865414645c6041733043c4081f8
msg357560 - (view) Author: miss-islington (miss-islington) Date: 2019-11-27 08:52
New changeset cd27d22ac90a869dc740004597246f24246348a6 by Miss Islington (bot) in branch '3.7':
bpo-38524: document implicit and explicit calling of descriptors' __set_name__ (GH-17364)
https://github.com/python/cpython/commit/cd27d22ac90a869dc740004597246f24246348a6
msg357561 - (view) Author: miss-islington (miss-islington) Date: 2019-11-27 08:53
New changeset 0f9c9d53283420a570850aa92869d032b40d4fba by Miss Islington (bot) in branch '3.8':
bpo-38524: document implicit and explicit calling of descriptors' __set_name__ (GH-17364)
https://github.com/python/cpython/commit/0f9c9d53283420a570850aa92869d032b40d4fba
msg357593 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-28 05:22
New changeset 02519f75d15b063914a11351da30178ca4ceb54b by Tal Einat in branch 'master':
bpo-38524: clarify example a bit and improve formatting (GH-17406)
https://github.com/python/cpython/commit/02519f75d15b063914a11351da30178ca4ceb54b
msg357598 - (view) Author: miss-islington (miss-islington) Date: 2019-11-28 05:28
New changeset 7e9bbbe51e74e5928e6a6c3e70434d824970ef58 by Miss Islington (bot) in branch '3.7':
bpo-38524: clarify example a bit and improve formatting (GH-17406)
https://github.com/python/cpython/commit/7e9bbbe51e74e5928e6a6c3e70434d824970ef58
msg357599 - (view) Author: miss-islington (miss-islington) Date: 2019-11-28 05:29
New changeset c0db88f6abbace79644b2aca2290bf41b1a37174 by Miss Islington (bot) in branch '3.8':
bpo-38524: clarify example a bit and improve formatting (GH-17406)
https://github.com/python/cpython/commit/c0db88f6abbace79644b2aca2290bf41b1a37174
msg357600 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-28 05:33
Thanks for the suggestion hongweipeng!

Thanks for the PR Florian, and congratulations on your first contribution to the project! May it be the first of many.
History
Date User Action Args
2019-11-28 05:33:36taleinatsetstatus: open -> closed
resolution: fixed
messages: + msg357600

stage: patch review -> resolved
2019-11-28 05:29:06miss-islingtonsetmessages: + msg357599
2019-11-28 05:28:42miss-islingtonsetmessages: + msg357598
2019-11-28 05:22:42miss-islingtonsetpull_requests: + pull_request16892
2019-11-28 05:22:34miss-islingtonsetpull_requests: + pull_request16891
2019-11-28 05:22:30taleinatsetmessages: + msg357593
2019-11-27 13:25:07taleinatsetpull_requests: + pull_request16886
2019-11-27 08:53:55miss-islingtonsetmessages: + msg357561
2019-11-27 08:52:42miss-islingtonsetnosy: + miss-islington
messages: + msg357560
2019-11-27 08:47:12miss-islingtonsetpull_requests: + pull_request16882
2019-11-27 08:47:06miss-islingtonsetpull_requests: + pull_request16881
2019-11-27 08:46:49taleinatsetmessages: + msg357559
2019-11-24 18:06:42taleinatsetversions: + Python 3.7, Python 3.9
2019-11-23 14:21:34DahlitzFloriansetkeywords: + patch
stage: patch review
pull_requests: + pull_request16849
2019-11-22 23:06:10taleinatsetmessages: + msg357334
2019-11-22 21:26:44DahlitzFloriansetnosy: + DahlitzFlorian
messages: + msg357325
2019-11-13 16:11:48taleinatsetkeywords: + easy
nosy: + taleinat
messages: + msg356539

2019-10-22 08:13:16sir-sigurdsetnosy: + sir-sigurd
2019-10-20 13:37:19ncoghlansetmessages: + msg355006
2019-10-20 13:30:54ncoghlansetmessages: + msg355004
2019-10-20 13:28:24ncoghlansetmessages: + msg355002
2019-10-19 14:58:30serhiy.storchakasetmessages: + msg354943
2019-10-19 14:55:55hongweipengsetmessages: + msg354942
2019-10-19 09:23:18serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg354934
2019-10-19 09:17:08serhiy.storchakasetnosy: + ncoghlan, carljm
2019-10-19 07:53:20steven.dapranosetnosy: + steven.daprano, docs@python
messages: + msg354930

assignee: docs@python
components: + Documentation
2019-10-19 07:30:51hongweipengcreate