classification
Title: setattr a read-only property; the AttributeError should show the attribute that failed
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Muhammad Alkarouri, abarry, apatrushev, ncoghlan, r.david.murray, rhettinger, uriyyo, xiang.zhang, xtreak, ztane
Priority: normal Keywords: patch

Created on 2016-08-18 13:57 by ztane, last changed 2020-12-30 09:52 by rhettinger. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23967 merged uriyyo, 2020-12-27 16:17
Messages (16)
msg273027 - (view) Author: Antti Haapala (ztane) * Date: 2016-08-18 13:57
Today we had an internal server error in production. I went to see the sentry logs for the error, and was dismayed: the error was `AttributeError: can't set attribute`, and the faulting line was `setattr(obj, attr, value)` that happens in a for-loop that uses computed properties coming from who knows.

Well, I quickly ruled out that it was because the code was trying to set a value to a read-only property descriptor, the only problem was to find out *which of all these read-only properties* was it trying to set:

Python 3.6.0a3+ (default, Aug 11 2016, 11:45:31) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class Foo:
...     @property
...     def bar(self): pass
... 
>>> setattr(Foo(), 'bar', 42)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: can't set attribute

Could we change this for Python 3.6 so that the message for this could include the name of the property just like `AttributeError: has no attribute 'baz'` does?
msg273033 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-08-18 14:39
If someone submits a patch in time, we'd probably accept it.
msg273035 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-08-18 14:51
The approach I'd take would be to change how {get,set,del}attr handle AttributeError; possibly by automatically filling in the information and giving a nicer error message. This would fix this as a side-effect (somewhat; some bits of code would need to change). How does that sound to core devs?
msg273036 - (view) Author: Antti Haapala (ztane) * Date: 2016-08-18 14:57
Unfortunately it seems that it is not that straightforward. The descriptor object doesn't know the name of the property. The error is raised in `property_descr_set`. However the error itself can be propagated from setting another property.
msg273302 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-08-21 16:28
Opened #27823 as a followup to this, and carried nosy list over.
msg273385 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-08-22 17:40
One solution I can think of is alter the constructor of property() and add an optional name attribute to it. If users really care about the exception message, they can set the attribute to the property's name. If they don't care, everything remains the same as now. This is just like the doc parameter of property() right now and the first parameter of type() when it is used to create a class dynamically. This is just a suggestion, I am not sure it's worth to do it.
msg273387 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-08-22 17:46
Not worth it.  It would feel like boilerplate, and the situation where you want the information is almost invariably going to be one you didn't anticipate and so didn't "bother" with the name.  Either that or you always have to do it, and the elegance of your python code takes a hit not commensurate with the benefit.
msg273396 - (view) Author: Antti Haapala (ztane) * Date: 2016-08-22 18:45
I've got one idea about how to implement this, but it would require adding a new flag field to PyExc_AttributeError type.

This flag, if set, would tell that the AttributeError in question was raised in C descriptor code or under similar circumstances, and that the attribute name was not known, and thus it is OK for setattr/delattr and attribute lookups to append ": attributename" to the end of the message, then clear the flag; then  all those places that raise AttributeError in __get__, __set__, __del__ would just need to set this flag.
msg301045 - (view) Author: Muhammad Alkarouri (Muhammad Alkarouri) Date: 2017-08-31 10:45
Now that the descriptor protocol has `__set_name__`, does this make any difference to this issue? The property can know its name, subject to a suitable patch.
msg383852 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-12-27 16:20
I have implemented this feature using an idea provided by Xiang Zhang.
Basically, property has new optional attribute `name` which will be added to msg of `AttributeError` in a case when `name` is set.
msg383868 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-12-27 21:41
Yurii, this looks nice.  I question whether *name* should be a part of the constructor because it immediately gets overridden by __set_name__ method.

>>> class A:
...    def x(self):
...        return 44
...    x = property(x, name='y')
...
>>> vars(A)['x'].name
'x'
msg383870 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-12-27 21:58
Raymond, it's a good question.

I have added `name` to `property` constructor to support cases when a property is added to a class after it was declared.

For instance:

```
class Foo:
    pass


Foo.foo = property(name='foo')

f = Foo()
f.foo = 10
```

So, in my opinion, it's expected behavior that `name` is overwritten by `__set_name__` method.

What do you think about that? Should we change this behevior?
msg383873 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-12-27 22:52
> In my opinion, it's expected behavior that `name` is 
> overwritten by `__set_name__` method.

It is almost certain that there will be others won't share that expectation.  The StackOverflow questions and bug reports are inevitable.

Most examples of __set_name__() use a private attribute.  I recommend that you go that route and stick to the spirit of the original bug report.  The OP asked for better error messages when possible.  They didn't ask for an API expansion.

> I have added `name` to `property` constructor to support cases
> when a property is added to a class after it was declared.

That rarely occurs in practice.  I wouldn't worry about it.  Also AFAICT the only time the new error message matters is in the context of a setattr() where the attribute isn't already shown in the traceback.  So the case in question is really a rarity inside another rarity.  Let's declare YAGNI unless an actual end-user problem arises in real-world code.
msg383876 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-12-27 23:23
You a totally right, looks like I tried to add a feature that no one asked for:)

I have updated PR and removed `name` parameter from `property` constuctor. Thanks for your advice.
msg384053 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-12-30 09:51
New changeset c56387f80c5aabf8100ceaffe365cc004ce0d7e0 by Yurii Karabas in branch 'master':
bpo-27794: Add `name` attribute to `property` class (GH-23967)
https://github.com/python/cpython/commit/c56387f80c5aabf8100ceaffe365cc004ce0d7e0
msg384054 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-12-30 09:52
Thanks for the PR :-)
History
Date User Action Args
2020-12-30 09:52:51rhettingersetstatus: open -> closed
versions: + Python 3.10, - Python 3.7
messages: + msg384054

resolution: fixed
stage: patch review -> resolved
2020-12-30 09:51:58rhettingersetmessages: + msg384053
2020-12-27 23:23:38uriyyosetmessages: + msg383876
2020-12-27 22:52:55rhettingersetmessages: + msg383873
2020-12-27 21:58:58uriyyosetmessages: + msg383870
2020-12-27 21:41:34rhettingersetassignee: rhettinger

messages: + msg383868
nosy: + rhettinger
2020-12-27 16:20:16uriyyosetmessages: + msg383852
2020-12-27 16:17:32uriyyosetkeywords: + patch
nosy: + uriyyo

pull_requests: + pull_request22812
stage: needs patch -> patch review
2018-07-26 20:49:21apatrushevsetnosy: + apatrushev
2018-07-25 10:02:25xtreaksetnosy: + xtreak
2017-11-09 14:29:32serhiy.storchakasetnosy: + ncoghlan
stage: needs patch

components: + Interpreter Core
versions: + Python 3.7
2017-11-09 14:20:41r.david.murraylinkissue31989 superseder
2017-08-31 10:45:19Muhammad Alkarourisetnosy: + Muhammad Alkarouri
messages: + msg301045
2016-08-22 18:45:52ztanesetmessages: + msg273396
2016-08-22 17:46:42r.david.murraysetmessages: + msg273387
2016-08-22 17:40:40xiang.zhangsetmessages: + msg273385
2016-08-21 16:28:34abarrysetmessages: + msg273302
2016-08-19 02:40:21xiang.zhangsetnosy: + xiang.zhang
2016-08-18 14:57:40ztanesetmessages: + msg273036
2016-08-18 14:51:08abarrysetnosy: + abarry
messages: + msg273035
2016-08-18 14:39:23r.david.murraysetnosy: + r.david.murray
messages: + msg273033
2016-08-18 13:57:19ztanecreate