Visitar URL original
Fix data descriptor detection in inspect.getattr_static · Issue #75367 · python/cpython · GitHub
Skip to content

Fix data descriptor detection in inspect.getattr_static #75367

Description

@davidhalter
BPO 31184
Nosy @rhettinger, @serhiy-storchaka
Superseder
  • bpo-26103: Contradiction in definition of "data descriptor" between (dotted lookup behavior/datamodel documentation) and (inspect lib/descriptor how-to)
  • Files
  • 0001-Fix-data-descriptor-detection-in-inspect.getattr_sta.patch: Potential patch that fixes the issue
  • 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

    Activity

    1. davidhalter commented on Aug 11, 2017

      davidhaltermannequin
      MannequinAuthor

      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.

    2. serhiy-storchaka commented on Aug 11, 2017

      @serhiy-storchaka
      Member

      See also bpo-26103.

    3. davidhalter commented on Dec 7, 2021

      davidhaltermannequin
      MannequinAuthor

      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.

    4. rhettinger commented on Dec 7, 2021

      @rhettinger
      Contributor

      Can you give an example of where getattr_static() is not doing what you expect?

    5. added
      stdlibStandard Library Python modules in the Lib/ directory
      and removed on Dec 7, 2021
    6. 7 remaining items

    7. davidhalter commented on Feb 21, 2023

      @davidhalter
      Author

      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.

    8. arhadthedev commented on Mar 4, 2023

      @arhadthedev
      Member
    9. davidhalter commented on Mar 5, 2023

      @davidhalter
      Author

      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.

    10. davidhalter commented on Mar 8, 2023

      @davidhalter
      Author

      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.

    11. removed
      pendingThe issue will be closed if no feedback is provided
      on Mar 8, 2023
    12. AlexWaygood commented on Apr 2, 2023

      @AlexWaygood
      Member

      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.

    13. carljm commented on Apr 6, 2023

      @carljm
      Member

      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.

    14. added a commit that references this issue on May 16, 2023
    15. added a commit that references this issue on May 16, 2023
    16. added a commit that references this issue on May 16, 2023
    17. added a commit that references this issue on May 17, 2023
    18. furkanonder commented on May 17, 2023

      @furkanonder
      Contributor

      @carljm The issue seems to have been resolved. We can close the issue.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.11only security fixesstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions