Skip to content

inspect.getmembers and inspect.classify_class_attrs mishandle descriptors #63230

Closed

Description

@ethanfurman
BPO 19030
Nosy @warsaw, @mjpieters, @ncoghlan, @pitrou, @bitdancer, @skrah, @ethanfurman, @koobs
Files
  • issue19030.stoneleaf.01.patch
  • issue19030.stoneleaf.02.patch
  • issue19030.stoneleaf.03.patch
  • issue19030.stoneleaf.04.patch
  • issue19030.stoneleaf.05.patch
  • koobs-freebsd9-amd64-py3x-build183.log
  • koobs-freebsd10-amd64-py3x-build588.log
  • koobs-freebsd9-py3x-build245.log
  • issue19030-without-docstrings.patch
  • 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 = 'https://github.com/ethanfurman'
    closed_at = <Date 2013-10-18.07:28:05.769>
    created_at = <Date 2013-09-16.00:00:54.563>
    labels = ['type-bug']
    title = 'inspect.getmembers and inspect.classify_class_attrs mishandle descriptors'
    updated_at = <Date 2014-02-25.21:05:20.551>
    user = 'https://github.com/ethanfurman'

    bugs.python.org fields:

    activity = <Date 2014-02-25.21:05:20.551>
    actor = 'r.david.murray'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2013-10-18.07:28:05.769>
    closer = 'python-dev'
    components = []
    creation = <Date 2013-09-16.00:00:54.563>
    creator = 'ethan.furman'
    dependencies = []
    files = ['31787', '31797', '31812', '31846', '32123', '32165', '32166', '32275', '32297']
    hgrepos = []
    issue_num = 19030
    keywords = ['patch']
    message_count = 42.0
    messages = ['197844', '197853', '197854', '197855', '197857', '197861', '197867', '197871', '197879', '197883', '197884', '197891', '197902', '197906', '198037', '198306', '198313', '198387', '198481', '198506', '199736', '199920', '199929', '200186', '200194', '200230', '200699', '200726', '200768', '200772', '200803', '200805', '200807', '200815', '200824', '200915', '200949', '200952', '200959', '200963', '212211', '212212']
    nosy_count = 11.0
    nosy_names = ['barry', 'mjpieters', 'ncoghlan', 'pitrou', 'r.david.murray', 'eli.bendersky', 'skrah', 'neologix', 'ethan.furman', 'python-dev', 'koobs']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19030'
    versions = ['Python 3.4']

    Activity

    1. ethanfurman commented on Sep 16, 2013

      @ethanfurman
      MemberAuthor

      Due to the odd nature of Enum classes and instances, the normal methods used by inspect.getmembers and inspect.classify_class_attrs are insufficient.

      By special casing Enum inside those two functions the correct information can be returned.

      Here is an example of the problem:

      =======================================================================================
      --> class Test(enum.Enum):
      ... this = 'that'
      ... these = 'those'
      ... whose = 'mine'
      ... name = 'Python'
      ... value = 'awesome'
      ... def what(self):
      ... return "%s is %s!" % (self.name, self.value)
      ...

      --> help(Test)
      Help on Test in module __main__ object:

      class Test(enum.Enum)
       |  Method resolution order:
       |      Test
       |      enum.Enum
       |      builtins.object
       |  
       |  Data and other attributes defined here:
       |  
       |  these = <Test.these: 'those'>
       |  
       |  this = <Test.this: 'that'>
       |  
       |  whose = <Test.whose: 'mine'>
       |  
       |  

      | Data descriptors inherited from enum.Enum:
      |
      | name
      | The name of the Enum member.
      |
      | value
      | The value of the Enum member.
      |
      | ----------------------------------------------------------------------
      | Data descriptors inherited from enum.EnumMeta:
      |
      | __members__
      | Returns a mapping of member name->value.
      |
      | This mapping lists all enum members, including aliases. Note that this
      | is a read-only view of the internal mapping.
      (END)
      =======================================================================================

      As can be seen, 'name' and 'value' are not showing up as enum members.

    2. bitdancer commented on Sep 16, 2013

      @bitdancer
      Member

      Special casing Enum in inspect has a code smell to it. There may not be a better option, but it sure feels ugly.

    3. ethanfurman commented on Sep 16, 2013

      @ethanfurman
      MemberAuthor

      Attached patch yields these results:

      =======================================================================================
      Help on class Test in module __main__:

      class Test(enum.Enum)
       |  Method resolution order:
       |      Test
       |      enum.Enum
       |      builtins.object
       |  
       |  Data and other attributes defined here:
       |  
       |  name = <Test.name: 'Python'>
       |  
       |  these = <Test.these: 'those'>
       |  
       |  this = <Test.this: 'that'>
       |  
       |  value = <Test.value: 'awesome'>
       |  
       |  whose = <Test.whose: 'mine'>
       |  
       |  

      | Data descriptors inherited from enum.Enum:
      |
      | name
      | The name of the Enum member.
      |
      | value
      | The value of the Enum member.
      |
      | ----------------------------------------------------------------------
      | Data descriptors inherited from enum.EnumMeta:
      |
      | __members__
      | Returns a mapping of member name->value.
      |
      | This mapping lists all enum members, including aliases. Note that this
      | is a read-only view of the internal mapping.
      (END)
      =======================================================================================

    4. ethanfurman commented on Sep 16, 2013

      @ethanfurman
      MemberAuthor

      R David Murray said:

      Special casing Enum in inspect has a code smell to it.

      I agree, and I'm certainly open to other options.

      The flow at this point is:

      help() --> inspect.classify_class_attrs --> dir() --> Enum.__dir__

      Because inspect relies on dir and Enum's dir has been customized, inspect will fail to return the whole story.

    5. bitdancer commented on Sep 16, 2013

      @bitdancer
      Member

      So the real problem is that inspect depends on dir? Isn't there already a bug open for that issue?

    6. ethanfurman commented on Sep 16, 2013

      @ethanfurman
      MemberAuthor

      I do not see one. I did post to PyDev asking about dir -- perhaps I should have given it a different title.

    7. ethanfurman commented on Sep 16, 2013

      @ethanfurman
      MemberAuthor

      Here's a crazy idea. :)

      The only reason the patch is tied to Enum is because of Enum's use of the _RouteClassAttributeToGetattr descriptor.

      If we had a module similar to functools, say classtools, we could flesh out _RouteClassAttributeToGetattr, rename it to VirtualAttribute, and then it would no longer be Enum specific.

    8. ncoghlan commented on Sep 16, 2013

      @ncoghlan
      Contributor

      types is the OO equivalent to functools these days, in addition to its original role of exposing the internal type objects that aren't builtins.

      However, it seems to me that the fix for bpo-1785 is simply *wrong*: it eliminated the exceptions at the expense of sometimes returning the *wrong answer*. The change in that issue means the inspect module isn't implementing the descriptor protocol correctly, and thus may provide a raw descriptor object when attempting to retrieve that attribute will return something else.

      Instead of the current behaviour, the inspect module should be trying getattr *first*, and only falling back to peeking in the __dict__ directly if that throws an exception.

    9. changed the title [-]Make inspect.getmembers and inspect.classify_class_attrs Enum aware[/-] [+]inspect.getmembers and inspect.classify_class_attrs mishandle descriptors[/+] on Sep 16, 2013
    10. pitrou commented on Sep 16, 2013

      @pitrou
      Member

      However, it seems to me that the fix for bpo-1785 is simply *wrong*: it eliminated the exceptions at the expense of sometimes returning the *wrong answer*.

      The underlying problem is that those functions are not very well-specified (actually, classify_class_attrs() is not specified at all: it's undocumented). The main consumer of inspect in the stdlib is pydoc, and pydoc being broken by third-party libraries with non-trivial descriptors was a major nuisance.

    11. ncoghlan commented on Sep 16, 2013

      @ncoghlan
      Contributor

      Right, we definitely want inspect to swallow the exceptions from
      descriptors. My suggestion is merely to switch the order to be:

      1. Try getattr
      2. If that throws an exception, check __dict__ directly
      3. If neither works (e.g. due to a buggy __dir__ method), ignore the
        attribute entirely.

      The problem at the moment is *working* descriptors that are designed to
      fall back on the metaclass lookup are being mishandled.

    12. pitrou commented on Sep 16, 2013

      @pitrou
      Member

      Right, we definitely want inspect to swallow the exceptions from
      descriptors. My suggestion is merely to switch the order to be:

      1. Try getattr
      2. If that throws an exception, check __dict__ directly
      3. If neither works (e.g. due to a buggy __dir__ method), ignore the
        attribute entirely.

      Are you talking about descriptors defined on the class or the metaclass? :-)

    13. changed the title [-]inspect.getmembers and inspect.classify_class_attrs mishandle descriptors[/-] [+]inspect.getmembers and inspect.classify_class_attrs mishandle descriptors[/+] on Sep 16, 2013
    14. ncoghlan commented on Sep 16, 2013

      @ncoghlan
      Contributor

      On the class, since that's the case which is breaking here (instances are
      already OK, since they trigger the success path in the custom descriptors)

    15. 21 remaining items

    16. python-dev commented on Oct 21, 2013

      python-devmannequin
      Mannequin

      New changeset dad1debba93c by Charles-François Natali in branch 'default':
      Fix test_pydoc failure introduced by 2f09a6980e1a (issue bpo-19030).
      http://hg.python.org/cpython/rev/dad1debba93c

    17. neologix commented on Oct 21, 2013

      neologixmannequin
      Mannequin

      I took the freedom to push a fix for the test_pydoc failures (all the buildbots were red). It should fix the failures, but since I'm not familiar with the code, it'd be good to have someone double-check it.

      Ethan, you just broke all the buildbots: you're now officialy a core developer! ;-)

    18. skrah commented on Oct 21, 2013

      skrahmannequin
      Mannequin

      The build --without-doc-strings still fails on the FreeBSD-9.0 bot.

      Antoine, could we make that option official on my build slave? Currently
      I'm subverting the build system by exporting an environment variable.

      It does not seem to be a heavy maintenance burden: This is the first
      failure in half a year or so.

    19. ethanfurman commented on Oct 21, 2013

      @ethanfurman
      MemberAuthor

      Charles-François Natali added the comment:

      Ethan, you just broke all the buildbots: you're now officialy a core
      developer! ;-)

      I was actually hoping to put off this particular honor for a while... drat! :)

    20. ethanfurman commented on Oct 21, 2013

      @ethanfurman
      MemberAuthor

      Thanks, Charles-François!

      The problem occurred when I tried to push the commit and was told there was trailing white-space. Naturally I then ran the de-whitespacing tool which of course removed the whitespace from those lines where you added the \x20s back on. I'll remember to do those conversions next time, and thanks for fixing it -- I'm not sure I would have found it any time soon.

    21. pitrou commented on Oct 21, 2013

      @pitrou
      Member

      Antoine, could we make that option official on my build slave? Currently
      I'm subverting the build system by exporting an environment variable.

      Done. Can you please watch for failures and ensure they get fixed?

    22. skrah commented on Oct 21, 2013

      skrahmannequin
      Mannequin

      Antoine Pitrou <report@bugs.python.org> wrote:

      Done. Can you please watch for failures and ensure they get fixed?

      Thanks! Yes, I'll keep an eye on it.

    23. skrah commented on Oct 22, 2013

      skrahmannequin
      Mannequin

      Here's the fix for --without-doc-strings (can't commit right now).

    24. ethanfurman commented on Oct 22, 2013

      @ethanfurman
      MemberAuthor

      Stefan, do the other tests in PydocWithMetaClasses continue to work on your FreeBSD 9.0 box without docstrings? Because they all fail on my Linux Ubuntu 13.04 box.

    25. skrah commented on Oct 22, 2013

      skrahmannequin
      Mannequin

      Ethan Furman <report@bugs.python.org> wrote:

      Stefan, do the other tests in PydocWithMetaClasses continue to work on your FreeBSD 9.0 box without docstrings? Because they all fail on my Linux Ubuntu 13.04 box.

      I tested on Debian, and the remaining tests seem to work (I did not bother to
      figure out why though). Let's just skip all tests if they fail for you.

    26. ethanfurman commented on Oct 22, 2013

      @ethanfurman
      MemberAuthor

      Actually, you @skipif clued me in as to what was happening with the other pydoc tests and their skipif clauses.

      Added appropriate skipifs to the new tests and mimicked the docstring in/exclusion. All tests now passing with and without docstrings, with and without -OO (simply skipped with -OO).

    27. python-dev commented on Oct 22, 2013

      python-devmannequin
      Mannequin

      New changeset 64d94b21e731 by Ethan Furman in branch 'default':
      Issue bpo-19030: fix new pydoc tests for --without-doc-strings
      http://hg.python.org/cpython/rev/64d94b21e731

    28. python-dev commented on Feb 25, 2014

      python-devmannequin
      Mannequin

      New changeset 4cd620d8c3f6 by R David Murray in branch 'default':
      whatsnew: DynanicClassAttribute (bpo-19030), Py_SetStandardStreamEncoding (bpo-16129)
      http://hg.python.org/cpython/rev/4cd620d8c3f6

    29. bitdancer commented on Feb 25, 2014

      @bitdancer
      Member

      I added docs for DynamicClassAttribute by copying the docstring. I think the doc entry could use some expansion, though, as it isn't obvious how to use it (or what, in fact, it does exactly).

    30. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions