Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed thePSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@jnsdrtlf

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please followthe steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

We also demand...A SHRUBBERY!

You cancheck yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

should we add a test case ?

Actually, from looking at the test failures... I'm not sure if this is the desired behavior (and the docs are ambiguous). Maybe@1st1 can shed some light on the "correct" behavior of this function?

Sinceinspect.getmembers is an introspection function it shouldn't run any actual code (i.e. executing getters). It should compute its result statically. Usinggetattr_static is the right idea here, for sure.

Okay then. So this probably doesn't need a new test, but the ~4 failing ones should be updated!

So this probably doesn't need a new test, but the ~4 failing ones should be updated!

I think it needs a new test. Basically an object with a property and a test ensuring that the property was discovered by getmembers, but the getter code wasn't run.

I think it needs a new test. Basically an object with a property and a test ensuring that the property was discovered by getmembers, but the getter code wasn't run.

Sounds good! I will get back to work and write a test later today.

The testtest_getmembers_VirtualAttribute expects this method

@types.DynamicClassAttributedefeggs(self):return'spam'

to be called. It asserts whether('eggs', 'spam') is ininspect.getmembers. But this is exactly the behavior that I tried to avoid with my PR.

There is also some wierd behaviour when overwriting__getattr__ in a metaclass. This probably needs more insight on howinspect.getmembers should behave.

Failing tests:

• test_get_slot_members
• test_getmembers_method
• test_getmembers_VirtualAttribute

Another inconsistent behaviour:inspect.isdatadescriptor should returnTrue for properties:

Return true if the object is a data descriptor.

Data descriptors have both aget and aset method. Examples are properties (defined in Python), getsets, and members

(Python Docs)

However, the following code does not return any properties:

classFoo:def__init__(self,data):self._bar=data@propertydefbar(self):print('--BAR--')returnself._barinspect.getmembers(Foo('test'),inspect.isdatadescriptor)

Result

--BAR--[]

Expected (new result)

[('__class__',<attribute'__class__'of'object'objects>), ('__dict__',<attribute'__dict__'of'Foo'objects>), ('__weakref__',<attribute'__weakref__'of'Foo'objects>), ('bar',<propertyobjectat0x...>)]

Is this correct so far? I am still confused about howgetmembers should behave. Seetest_getmembers_VirtualAttribute, this test expects a different behaviour.

Additionally,

classFoo:passinspect.getattr_static(Foo('test'),'__class__')

returns<attribute '__class__' of 'object' objects> whereasgetattr would return<class '__main__.Foo'>. What shouldgetmembers return? If it returns<attribute '__class__' of 'object' objects>, theinspect.isclass predicate would not work:

classFoo:pass# Will return [] if getattr_static is usedinspect.getmembers(Foo('test'),inspect.isclass)

In3ac2093, I've changedinspect.getmembers to only useinspect.getattr_static for properties, as other members (e.g.__class__) are expected to be returned usinggetattr(obj,'__class__'):

getattr_static(obj,'__class__') returns<attribute '__class__' of 'object' objects> whereasgetattr would return<class '__main__.Foo'>

This feels more like a temporary fix - could be reverted to the first fix I've committed - although I am just not sure which is better.

At this point,I am not sure what the actual expected behaviour should be. Someone should review this and the test case that is currently failing, as it expects a different behaviour (return the actual value of the property).

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@1st1 I share your thoughts on that. It does not seem to be obvious, whatgetmembers should return - and changing it in any way will break existing code. This has already caused me a lot of headaches.

As for introducing a functiongetmembers_static: I still have some questions that should be discussed before doing so:

1. Whatshouldgetmembers_static return? Isgetattr_static appropriate for every member? This would break some predicates (e.g.isclass, see comment above). There does not seem to be a "simple" solution.

2. A technical question: The new method should pass most of the same tests asgetmembers. Would one just copy and paste the old ones? It would also share most of the code - except for maybe 1 line - withgetmembers.

3. Is there a reason forgetmembers to return a list of tuples? I would have suggested to use adict instead, which feels more intuitive on first though.

Is there a better place to discuss such questions?

Is there a better place to discuss such questions?

I think it would be better if you could post this to the python-ideas mailing list. I've very limited time and would hate to get this thing staled because of me.

Or post it here:https://discuss.python.org/c/ideas

I added a link to the discussion on the bug tracker.

This would be very helpful, running into the same issue with inspect.getmembers evaluating@Property decorated methods.

getmembers_static would solve it.