GeezerVison is showing github.com — Fix data descriptor detection in inspect.getattr_static · Issue #75367 · python/cpython
Fix data descriptor detection in inspect.getattr_static #75367Description
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 = None
closed_at = None
created_at = <Date 2017-08-11.16:22:11.131>
labels = ['type-bug', 'library', '3.11']
title = 'Fix data descriptor detection in inspect.getattr_static'
updated_at = <Date 2021-12-08.04:22:58.232>
user = 'https://bugs.python.org/davidhalter' bugs.python.org fields:
activity = <Date 2021-12-08.04:22:58.232>
actor = 'rhettinger'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2017-08-11.16:22:11.131>
creator = 'davidhalter'
dependencies = []
files = ['47078']
hgrepos = []
issue_num = 31184
keywords = ['patch']
message_count = 4.0
messages = ['300170', '300173', '407976', '407985']
nosy_count = 3.0
nosy_names = ['rhettinger', 'serhiy.storchaka', 'davidhalter']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'open'
superseder = '26103'
type = 'behavior'
url = 'https://bugs.python.org/issue31184'
versions = ['Python 3.11'] Linked PRs
Activitydavidhalter commented on Aug 11, 2017 MannequinAuthor More actions 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 commented on Aug 11, 2017 davidhalter commented on Dec 7, 2021 MannequinAuthor More actions 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 commented on Dec 7, 2021 Can you give an example of where getattr_static() is not doing what you expect? davidhalter commented on 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 commented on Mar 4, 2023 davidhalter commented on 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 commented on Mar 8, 2023 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_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. AlexWaygood commented on Apr 2, 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 than 42.
If you file a PR for this issue, feel free to ping me on it -- I'd be happy to review it. carljm commented on 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 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. furkanonder commented on May 17, 2023 @carljm The issue seems to have been resolved. We can close the issue. Reacted by Alex Waygood and Carl Meyer
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|