Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspect.classify_class_attrs ignores metaclass #63129

Closed
ethanfurman opened this issue Sep 5, 2013 · 11 comments
Closed

inspect.classify_class_attrs ignores metaclass #63129

ethanfurman opened this issue Sep 5, 2013 · 11 comments
Assignees
Labels
type-bug An unexpected behavior, bug, or error

Comments

@ethanfurman
Copy link
Member

BPO 18929
Nosy @pitrou, @ethanfurman
Files
  • local_fix.stoneleaf.01
  • global_fix.stoneleaf.01
  • issue18929.stoneleaf.03.patch
  • issue18929.stoneleaf.04.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-09-15.01:53:45.894>
    created_at = <Date 2013-09-05.02:51:02.618>
    labels = ['type-bug']
    title = 'inspect.classify_class_attrs ignores metaclass'
    updated_at = <Date 2013-09-15.01:53:45.892>
    user = 'https://github.com/ethanfurman'

    bugs.python.org fields:

    activity = <Date 2013-09-15.01:53:45.892>
    actor = 'python-dev'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2013-09-15.01:53:45.894>
    closer = 'python-dev'
    components = []
    creation = <Date 2013-09-05.02:51:02.618>
    creator = 'ethan.furman'
    dependencies = []
    files = ['31594', '31595', '31693', '31706']
    hgrepos = []
    issue_num = 18929
    keywords = ['patch']
    message_count = 11.0
    messages = ['196973', '196980', '197037', '197162', '197280', '197285', '197335', '197346', '197356', '197417', '197742']
    nosy_count = 4.0
    nosy_names = ['pitrou', 'eli.bendersky', 'ethan.furman', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18929'
    versions = ['Python 3.4']

    @ethanfurman
    Copy link
    Member Author

    Part of the solution for bpo-18693 is to have inspect.classify_class_attrs() properly consider the metaclass (or type) of the class when searching for the origination point of class attributes.

    The fix is changing line 325:

    •    for base in (cls,) + mro:
      

    + for base in (cls,) + mro + (type(cls),):

    or line 361:

    • return cls.__mro__
      + return cls.__mro__ + (type(cls), )

    Should we target previous Pythons with this fix?

    @ethanfurman ethanfurman added the type-bug An unexpected behavior, bug, or error label Sep 5, 2013
    @ethanfurman
    Copy link
    Member Author

    The global fix causes these two tests to fail:

    ======================================================================
    FAIL: test_newstyle_mro (test.test_inspect.TestClassesAndFunctions)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/ethan/source/python/issue18693/Lib/test/test_inspect.py", line 485, in test_newstyle_mro
        self.assertEqual(expected, got)
    AssertionError: Tuples differ: (<class 'test.test_inspect.Tes... != (<class 'test.test_inspect.Tes...

    Second tuple contains 1 additional elements.
    First extra element 5:
    <class 'type'>

    (<class 'test.test_inspect.TestClassesAndFunctions.test_newstyle_mro.<locals>.D'>,
    <class 'test.test_inspect.TestClassesAndFunctions.test_newstyle_mro.<locals>.B'>,
    <class 'test.test_inspect.TestClassesAndFunctions.test_newstyle_mro.<locals>.C'>,
    <class 'test.test_inspect.TestClassesAndFunctions.test_newstyle_mro.<locals>.A'>,

    • <class 'object'>)
      ? ^

    + <class 'object'>,
    ? ^

    + <class 'type'>)

    ======================================================================
    FAIL: test_help_output_redirect (test.test_pydoc.PydocDocTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/ethan/source/python/issue18693/Lib/test/test_pydoc.py", line 396, in test_help_output_redirect
        self.assertEqual(expected_text, result)
    AssertionError: "Help on module test.pydoc_mod in test:\n\nNAME\n    test.pydoc_mod - This is a  [truncated]... != "Help on module test.pydoc_mod in test:\n\nNAME\n    test.pydoc_mod - This is a  [truncated]...
      Help on module test.pydoc_mod in test:
      
      NAME
          test.pydoc_mod - This is a test module for test_pydoc
      
      CLASSES
          builtins.object
              A
              B
          
          class A(builtins.object)
           |  Hello and goodbye
    +      |  
    +      |  Method resolution order:
    +      |      A
    +      |      builtins.object
    +      |      builtins.type
           |  
           |  Methods defined here:
           |  
           |  __init__()
           |      Wow, I have no function!
           |  
           |  

       |  Data descriptors defined here:
       |  
       |  \_\_dict__
       |      dictionary for instance variables (if defined)
       |  
       |  \_\_weakref__
       |      list of weak references to the object (if defined)
      
      class B(builtins.object)
    

    + | Method resolution order:
    + | B
    + | builtins.object
    + | builtins.type
    + |
    | Data descriptors defined here:
    |
    | __dict__
    | dictionary for instance variables (if defined)
    |
    | __weakref__
    | list of weak references to the object (if defined)
    |
    | ----------------------------------------------------------------------
    | Data and other attributes defined here:
    |
    | NO_MEANING = 'eggs'

    ======================================================================

    I suspect (hope) updating the tests would be fine.

    @ethanfurman
    Copy link
    Member Author

    Another option with the global fix is to only add the metaclass to the mro if the metaclass is not 'type'.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 7, 2013

    Of course it is causing tests to fail. You are changing behaviour of an existing API. Please find another solution.

    @ethanfurman
    Copy link
    Member Author

    Okay, taking a step back.

    It seems that currently inspect is geared towards instances and classes, not metaclasses. Consequently, so is help.

    So, how do we enhance inspect so that help can be metaclass aware?

    classify_class_attrs seems like an obvious choice, and it's docstring currently says this:

    def classify_class_attrs(cls):
        """Return list of attribute-descriptor tuples.
    For each name in dir(cls), the return list contains a 4-tuple
    with these elements:
    
        0. The name (a string).
    
        1. The kind of attribute this is, one of these strings:
               'class method'    created via classmethod()
               'static method'   created via staticmethod()
               'property'        created via property()
               'method'          any other flavor of method
               'data'            not a method
    
        2. The class which defined this attribute (a class).
    
        3. The object as obtained directly from the defining class's
           __dict__, not via getattr.  This is especially important for
           data attributes:  C.data is just a data object, but
           C.__dict__['data'] may be a data descriptor with additional
           info, like a __doc__ string.
    """
    

    We could add additional 'kind' of 'hidden class method', and 'hidden class attributes' and then have classify_class_attrs explicitly search metaclasses.

    Or, since we have to make a new getmembers (getmetaclassmembers?), we could also make a new classify_metaclass_attrs that handled both class and metaclass.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 8, 2013

    It seems that currently inspect is geared towards instances and
    classes, not metaclasses. Consequently, so is help.

    I'm afraid I don't really understand what you're talking about. A
    metaclass is just a slightly different kind of class :-)

    def classify_class_attrs(cls):
    """Return list of attribute-descriptor tuples.

    For each name in dir(cls), the return list contains a 4-tuple
    with these elements:
    

    [...]

    We could add additional 'kind' of 'hidden class method', and 'hidden
    class attributes' and then have classify_class_attrs explicitly search
    metaclasses.

    The docstring is clear: "For each name in dir(cls)". If you want stuff
    that hidden's in the cls.__class__ (and, consequently, not in dir(cls)),
    then you are not looking for the right function, I think.

    I can understand wanting to better automate lookup of methods on
    classes, rather than on instances, but it would probably deserve another
    function.

    But I have another question first: doesn't calling
    classify_class_attrs() on the metaclass already do what you want?

    @ethanfurman
    Copy link
    Member Author

    Antoine, to answer all your questions at once, and using an Enum as the example:

    --> dir(Color)
    ['__class__', '__doc__', '__members__', '__module__', 'blue', 'green', 'red']

    --> Color.__members__
    mappingproxy(OrderedDict([('red', <Color.red: 1>), ('green', <Color.green: 2>), ('blue', <Color.blue: 3>)]))

    -->help(Color)
    ========================================
    Help on class Color in module __main__:

    Color = <enum 'Color'>
    ========================================

    Why? Because __members__ is not in Color.__dict__

    --> Color.__dict__.keys()
    dict_keys(['_member_type_', '_member_names_', '__new__', '_value2member_map_', '__module__', '_member_map_', '__doc__'])

    and inspect.classify_class_attrs doesn't look in metaclasses, instead assigning None as the class if it can't find it:

    --> inspect.classify_class_attrs(Color) # output trimmed for brevity
    Attribute(name='__members__', kind='data', defining_class=None, object= ... )

    So, even though __members__ does show up in dir(Color), classify_class_attrs incorrectly assigns its class.

    Can I use classify_class_attrs on Color.__class__? Sure, but then I get 50 items, only one of which was listed in dir(Color).

    Interestingly, getmembers(Color) does return __members__, even though it is a metaclass attribute and the docs warn that it won't:

    --> print('\n'.join([str(i) for i in inspect.getmembers(Color)]))
    ('__class__', <attribute '__class__' of 'object' objects>)
    ('__doc__', None)
    ('__members__', mappingproxy(OrderedDict([('red', <Color.red: 1>), ('green', <Color.green: 2>), ('blue', <Color.blue: 3>)])))
    ('__module__', '__main__')
    ('blue', <Color.blue: 3>)
    ('green', <Color.green: 2>)
    ('red', <Color.red: 1>)

    So if getmembers() correctly reports it because it shows up in dir(), then classify_class_attrs should correctly identify it as well.

    @ethanfurman
    Copy link
    Member Author

    This is a more useful help() -- patch attached.

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

    class Color(enum.Enum)
     |  Method resolution order:
     |      Color
     |      enum.Enum
     |      builtins.object
     |  
     |  Data and other attributes defined here:
     |  
     |  blue = <Color.blue: 3>
     |  
     |  green = <Color.green: 2>
     |  
     |  red = <Color.red: 1>
     |  
     |  

    | 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.

    @pitrou
    Copy link
    Member

    pitrou commented Sep 9, 2013

    So, even though __members__ does show up in dir(Color),
    classify_class_attrs incorrectly assigns its class.

    [...]

    So if getmembers() correctly reports it because it shows up in dir(),
    then classify_class_attrs should correctly identify it as well.

    Ok, you got me convinced :-)

    @ethanfurman
    Copy link
    Member Author

    Cool. Latest patch has a slight doc fix for getmembers.

    Will commit on Friday if no other pertinent feedback.

    @ethanfurman ethanfurman self-assigned this Sep 10, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 15, 2013

    New changeset b0517bb271ad by Ethan Furman in branch 'default':
    Close bpo-18929: inspect.classify_class_attrs will now search the metaclasses (last) to find where an attr was defined.
    http://hg.python.org/cpython/rev/b0517bb271ad

    @python-dev python-dev mannequin closed this as completed Sep 15, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants