You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix data descriptor detection in inspect.getattr_static #75367
bpo-26103: Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to)
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=Noneclosed_at=Nonecreated_at=<Date2017-08-11.16:22:11.131>labels= ['type-bug', 'library', '3.11']
title='Fix data descriptor detection in inspect.getattr_static'updated_at=<Date2021-12-08.04:22:58.232>user='https://bugs.python.org/davidhalter'
This is not a duplicate. It is related to https://bugs.python.org/issue26103, because __get__ is not required anymore for an object to be a data descriptor. The current code on master (of inspect.getattr_static) still thinks a descriptor has both __get__ and __set__ set.
Since issue bpo-26103 has been fixed, it's now clear that my patch is slightly wrong, but I'm happy to fix that if someone is actually going to review it.
getattr get 42
getattr_static get 42
getattr get_delete Foo
getattr_static get_delete 42
getattr get_set Foo
getattr_static get_set <__main__.DescriptorGetSet object at 0x7f88b10ab7f0>
The get_delete case is clearly wrong, because if a normal getattr returns the executed descriptor, we should never get the value of the instance's __dict__ as a result in the getattr_static case.
@iritkatriel@rhettinger Does this make sense? In case you think it's a bug as well, I'm happy to create a pull request with my patch & a unit test of the __delete__ case.
The fix here is simply to also check for __delete__ method, not only __set__ method.
To be clear, despite the resolution of #70291 (which I think was resolved incorrectly), the check for __get__ method here in getattr_static is correct and should remain. In other words, I think @davidhalter 's original patch here remains the correct fix, despite the resolution of #70291.
Activity
davidhalter commentedon Aug 11, 2017
inspect.getattr_static is currently not identifying data descriptors the right way.
Data descriptors are defined by having a __get__ attribute and at least one of the __set__ and __delete__ attributes.
Implementation detail: Both __delete__ and __get__ set the same slot called tp_descr_set in CPython.
I have attached a patch that fixes the issue IMO.
serhiy-storchaka commentedon Aug 11, 2017
See also bpo-26103.
davidhalter commentedon Dec 7, 2021
This is not a duplicate. It is related to https://bugs.python.org/issue26103, because __get__ is not required anymore for an object to be a data descriptor. The current code on master (of inspect.getattr_static) still thinks a descriptor has both __get__ and __set__ set.
Since issue bpo-26103 has been fixed, it's now clear that my patch is slightly wrong, but I'm happy to fix that if someone is actually going to review it.
rhettinger commentedon Dec 7, 2021
Can you give an example of where getattr_static() is not doing what you expect?
7 remaining items
davidhalter commentedon Feb 21, 2023
Thanks for bringing it up again. I unfortunately somehow missed @rhettinger's comment on this issue. Will try to give an example as soon as possible.
arhadthedev commentedon Mar 4, 2023
@davidhalter ping
davidhalter commentedon Mar 5, 2023
As soon as possible doesn't mean I have time immediately. It will happen and it's still on my radar. Sorry, but I have been quite busy.
davidhalter commentedon Mar 8, 2023
Here's my example:
Output:
The
get_deletecase is clearly wrong, because if a normal getattr returns the executed descriptor, we should never get the value of the instance's__dict__as a result in thegetattr_staticcase.@iritkatriel @rhettinger Does this make sense? In case you think it's a bug as well, I'm happy to create a pull request with my patch & a unit test of the
__delete__case.AlexWaygood commentedon Apr 2, 2023
Thanks @davidhalter. I agree that this looks like a bug. Just to give a slightly shorter reproducer:
The result of the last call should probably be
<__main__.DescriptorGetDelete object at 0x000001D9BF998DA0>rather than42.If you file a PR for this issue, feel free to ping me on it -- I'd be happy to review it.
carljm commentedon Apr 6, 2023
The fix here is simply to also check for
__delete__method, not only__set__method.To be clear, despite the resolution of #70291 (which I think was resolved incorrectly), the check for
__get__method here ingetattr_staticis correct and should remain. In other words, I think @davidhalter 's original patch here remains the correct fix, despite the resolution of #70291.Merge branch 'main' into pythongh-75367
Merge branch 'main' into pythongh-75367
gh-75367: Fix data descriptor detection in inspect.getattr_static (#1…
pythongh-75367: Fix data descriptor detection in inspect.getattr_stat…
[3.11] gh-75367: Fix data descriptor detection in inspect.getattr_sta…
Merge branch 'main' into 709funconly
furkanonder commentedon May 17, 2023
@carljm The issue seems to have been resolved. We can close the issue.