What is the purpose of iterating the instance attribute __bases__?

@serhiy-storchaka
AFAIK To handle this case,

import inspect
import types

class Base(object):
    @types.DynamicClassAttribute
    def fried(self):
        return self._x
    _x = object()

class A(Base):
    @types.DynamicClassAttribute
    def eggs(self):
        return self._x
    _x = object()

inspect.getmembers(A)

For reviewers,

__bases__ is a tuple containing the base classes from https://docs.python.org/3/reference/datamodel.html

I add the check the type of object.__bases__.

@serhiy-storchaka
But I don't know exactly why this is needed.
The first reference is here. e03ea37#diff-8fb5d890c8ae80e19883adc7fbdd2c8fR337

I found the case of the related to this code.

"Lib/test/test_enum.py", line 2560, in test_inspect_getmembers

I think that we can update this code into

@@ -331,7 +331,8 @@ def getmembers(object, predicate=None):
             for base in object.__bases__:
                 for k, v in base.__dict__.items():
                     if isinstance(v, types.DynamicClassAttribute):
-                        names.append(k)
+                        if k not in names:
+                            names.append(k)
     except AttributeError:

@serhiy-storchaka

Current implementation returns dulplicated members,

, ('eggs', <types.DynamicClassAttribute object at 0x10ed38970>), ('fried', <types.DynamicClassAttribute object at 0x10ed38940>), ('fried', <types.DynamicClassAttribute object at 0x10ed38940>)]

But with this PR

('_x', <object object at 0x10e1691a0>), ('eggs', <types.DynamicClassAttribute object at 0x10e2c0970>), ('fried', <types.DynamicClassAttribute object at 0x10e2c0940>)]

It works well.

Shouldn't this be executed only for classes?

@serhiy-storchaka

import enum
import inspect

class Color(enum.Enum):
    red = 1
    green = 2
    blue = 3

values = dict((
    ('__class__', enum.EnumMeta),
    ('__doc__', 'An enumeration.'),
    ('__members__', Color.__members__),
    ('__module__', __name__),
    ('blue', Color.blue),
    ('green', Color.green),
    ('name', enum.Enum.__dict__['name']),
    ('red', Color.red),
    ('value', enum.Enum.__dict__['value']),
    ))

print(dict(inspect.getmembers(Color)).keys())
c = Color.red
print(c.value)

Here is the case,
Without traversing __base__, it can not get value and name from inspect.getmembers.

In all your examples object is a class.

@serhiy-storchaka
I am now understanding!
I've updated to check the object is class before traversing.

Current implementation returns dulplicated members,

The comment above the loop over object.__bases__ explicitly says about duplicated entries. And the code below handles them.

@serhiy-storchaka Thanks I've updated.

@serhiy-storchaka
Thanks, I've updated the documentation and the unit test.

@1st1

Thank you for your review despite the busy situation.
Following your guide, I 've added a configuration to my .vimrc also! Thanks!

 set colorcolumn=79
 autocmd BufWritePre *.py :%s/\s\+$//e

I 've reflected your review.
PTAL

@1st1, @serhiy-storchaka

@serhiy-storchaka Are you sure you want to backport this to 2.7? I'm not; it might accidentally fix some things that weren't working for a decade and there are now workarounds in place that might fail if we fix this.

Isn't this true for 3.6 and 3.7 as well, except for the "decade" part?

IMO this should only go into 3.8, and have a "What's New" entry mentioning the change.

This strikes me as being similar to bpo-33899, in that this is probably how it should have been to begin with, but changing it now in released versions would likely unexpectedly break existing code that uses it.

This PR should be backported at least to 3.7, because a module-level __getattr__ is new in 3.7, and this increase the chance to encounter this bug.

But the bug can be encountered in older Python versions for classes with __getattr__. I don't think that fixing it will break somebody's code. Do you have any examples?

Do you have any examples?

No, I don't. You're probably right about backporting this to 3.x.

@serhiy-storchaka @taleinat

Thanks for the review! I've updated the PR.

@1st1
Can you take a look, please?

Could you please add a test for your last change? I.e. the test that passed when iterate mro, but fails when iterate __bases__. It can be a class with indirectly inherited DynamicClassAttribute.

class A: # A DynamicClassAttribute should be defined here
class B(A): pass
class C(B): pass
# test inspect.getmembers(C)

@serhiy-storchaka
Thanks
I 've added a test test_getmembers_with_dynamic_class_attribute which was failed when
iterate __bases__.

@serhiy-storchaka

I updated the unit test and to understand more deeply.
Please reference below script and outputs.
As I am not an expert on inspect module. I don't know which is right output.

import types
import inspect


class A:
    @types.DynamicClassAttribute
    def ham(self):
        return 'eggs'

class B(A): pass
class C(B): pass

print([a for a in inspect.getmembers(A) if a[0] == 'ham'])
print([b for b in inspect.getmembers(B) if b[0] == 'ham'])
print([c for c in inspect.getmembers(C) if c[0] == 'ham'])

AS-IS -> iterate __bases__

[('ham', <types.DynamicClassAttribute object at 0x10d2f8198>)]
[('ham', <types.DynamicClassAttribute object at 0x10d2f8198>), ('ham', <types.DynamicClassAttribute object at 0x10d2f8198>)]
[('ham', <types.DynamicClassAttribute object at 0x10d2f8198>)]

TO-BE 98d79c7 -> iterate mro

[('ham', <types.DynamicClassAttribute object at 0x103365130>), ('ham', <types.DynamicClassAttribute object at 0x103365130>)]
[('ham', <types.DynamicClassAttribute object at 0x103365130>), ('ham', <types.DynamicClassAttribute object at 0x103365130>)]
[('ham', <types.DynamicClassAttribute object at 0x103365130>), ('ham', <types.DynamicClassAttribute object at 0x103365130>)]

Should not inspect.getmembers() remove duplicates?

Seems using mro instead of __bases__ doesn't fix any bug. In this case it is better to restore the original code.

@serhiy-storchaka
I revert the code to use __bases__.
But I was wondering if this PR was the right way.
Do you think this PR would not be merged and should we find out another way?
If so, I am going to close this PR :)

Maybe other good people will find the way.

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

@1st1 Can you take a look, please?

@1st1 @serhiy-storchaka @vstinner @csabella

Hi, this PR remains for a year.
Is there any plan with this PR?

Thanks

@1st1 @serhiy-storchaka @vstinner @csabella

I close this PR due to no feedback..
If there is any chance to work with this issue.
I will reopen this PR

Thanks!