Repository navigation
Fix data descriptor detection in inspect.getattr_static #75367
Description
Activity
davidhalter commented
on Aug 11, 2017 davidhaltermannequinMannequinAuthorMore actionsinspect.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.
See also bpo-26103.
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.
- added3.8 (EOL)end of lifeend of life3.9 (EOL)end of lifeend of life3.10 (EOL)end of lifeend of life3.11only security fixesonly security fixes
on Dec 7, 2021 Can you give an example of where getattr_static() is not doing what you expect?
- addedstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directoryand removed3.7 (EOL)end of lifeend of life3.8 (EOL)end of lifeend of life3.9 (EOL)end of lifeend of life
on Dec 7, 2021 7 remaining items
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.
@davidhalter ping
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.
Here's my example:
import inspect class DescriptorGet: def __get__(self, instance, klass): return "Foo" class DescriptorGetDelete: def __get__(self, instance, klass): return "Foo" def __delete__(self, instance, klass): pass class DescriptorGetSet: def __get__(self, instance, klass): return "Foo" def __set__(self, instance, klass, value): pass class Foo: get = DescriptorGet() get_delete = DescriptorGetDelete() get_set = DescriptorGetSet() def __init__(self): self.__dict__['get'] = 42 self.__dict__['get_delete'] = 42 self.__dict__['get_set'] = 42 for attr in ['get', 'get_delete', 'get_set']: print(f'getattr {attr}', getattr(Foo(), attr)) print(f'getattr_static {attr}', inspect.getattr_static(Foo(), attr))Output:
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_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.- removedpendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided
on Mar 8, 2023 Thanks @davidhalter. I agree that this looks like a bug. Just to give a slightly shorter reproducer:
>>> class DescriptorGetDelete: ... def __get__(self, instance, klass): ... return 'foo' ... def __delete__(self, instance, klass): pass ... >>> class Foo: ... get_delete = DescriptorGetDelete() ... def __init__(self): ... self.__dict__['get_delete'] = 42 ... >>> foo = Foo() >>> foo.get_delete 'foo' >>> import inspect >>> inspect.getattr_static(foo, 'get_delete') 42
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.
Reacted by Carl MeyerThe 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.- added a commit that references this issue
on May 16, 2023 - added a commit that references this issue
on May 16, 2023 - added a commit that references this issue
on May 17, 2023 @carljm The issue seems to have been resolved. We can close the issue.
Reacted by Alex Waygood and Carl Meyer
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:
bugs.python.org fields:
Linked PRs