classification
Title: inspect.classify_class_attrs ignores metaclass
Type: behavior Stage: resolved
Components: Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: eli.bendersky, ethan.furman, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2013-09-05 02:51 by ethan.furman, last changed 2013-09-15 01:53 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
local_fix.stoneleaf.01 ethan.furman, 2013-09-05 02:51 review
global_fix.stoneleaf.01 ethan.furman, 2013-09-05 02:52 review
issue18929.stoneleaf.03.patch ethan.furman, 2013-09-09 04:58 review
issue18929.stoneleaf.04.patch ethan.furman, 2013-09-10 02:53 review
Messages (11)
msg196973 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-05 02:51
Part of the solution for Issue18693 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?
msg196980 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-05 05:58
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.
msg197037 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-06 00:07
Another option with the global fix is to only add the metaclass to the mro if the metaclass is not 'type'.
msg197162 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-07 13:43
Of course it is causing tests to fail. You are changing behaviour of an existing API. Please find another solution.
msg197280 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-08 16:39
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.
msg197285 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-08 16:49
> 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?
msg197335 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-08 23:21
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.
msg197346 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-09 04:58
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.
msg197356 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-09 08:34
> 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 :-)
msg197417 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-10 02:53
Cool.  Latest patch has a slight doc fix for getmembers.

Will commit on Friday if no other pertinent feedback.
msg197742 - (view) Author: Roundup Robot (python-dev) Date: 2013-09-15 01:53
New changeset b0517bb271ad by Ethan Furman in branch 'default':
Close #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
History
Date User Action Args
2013-09-15 01:53:45python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg197742

resolution: fixed
stage: patch review -> resolved
2013-09-10 02:53:39ethan.furmansetfiles: + issue18929.stoneleaf.04.patch
assignee: ethan.furman
messages: + msg197417

stage: patch review
2013-09-09 08:34:17pitrousetmessages: + msg197356
2013-09-09 04:58:27ethan.furmansetfiles: + issue18929.stoneleaf.03.patch
keywords: + patch
messages: + msg197346
2013-09-08 23:21:09ethan.furmansetmessages: + msg197335
2013-09-08 16:49:38pitrousetmessages: + msg197285
2013-09-08 16:39:36ethan.furmansetmessages: + msg197280
2013-09-07 13:43:27pitrousetnosy: + pitrou
messages: + msg197162
2013-09-06 00:07:05ethan.furmansetmessages: + msg197037
2013-09-05 05:58:44ethan.furmansetmessages: + msg196980
2013-09-05 02:52:12ethan.furmansetfiles: + global_fix.stoneleaf.01
2013-09-05 02:51:59ethan.furmansetfiles: + local_fix.stoneleaf.01
2013-09-05 02:51:02ethan.furmancreate