[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Thanks for the PR :-)

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
versions: +Python 3.10 -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Raymond Hettinger]

Raymond Hettinger  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Yurii Karabas]

Yurii Karabas <1998uri...@gmail.com> added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Yurii Karabas]

Yurii Karabas <1998uri...@gmail.com> added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Raymond Hettinger]

Raymond Hettinger  added the comment:

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'

--
assignee:  -> rhettinger
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Yurii Karabas]

Yurii Karabas <1998uri...@gmail.com> added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Yurii Karabas]

Change by Yurii Karabas <1998uri...@gmail.com>:


--
keywords: +patch
nosy: +uriyyo
nosy_count: 8.0 -> 9.0
pull_requests: +22812
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/23967

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Anton Patrushev]

Change by Anton Patrushev :


--
nosy: +apatrushev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
nosy: +xtreak

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
components: +Interpreter Core
nosy: +ncoghlan
stage:  -> needs patch
versions: +Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Muhammad Alkarouri]

Muhammad Alkarouri added the comment:

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.

--
nosy: +Muhammad Alkarouri

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Antti Haapala]

Antti Haapala added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Xiang Zhang]

Xiang Zhang added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[R. David Murray]

R. David Murray added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Emanuel Barry]

Emanuel Barry added the comment:

Opened #27823 as a followup to this, and carried nosy list over.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Xiang Zhang]

Changes by Xiang Zhang :


--
nosy: +xiang.zhang

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Antti Haapala]

Antti Haapala added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Emanuel Barry]

Emanuel Barry added the comment:

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?

--
nosy: +ebarry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[R. David Murray]

R. David Murray added the comment:

If someone submits a patch in time, we'd probably accept it.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27794] setattr a read-only property; the AttributeError should show the attribute that failed

[Antti Haapala]

New submission from Antti Haapala:

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 "", line 1, in 
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?

--
messages: 273027
nosy: ztane
priority: normal
severity: normal
status: open
title: setattr a read-only property; the AttributeError should show the 
attribute that failed
type: enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com