GeezerVison

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 the PSF 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 follow the 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 can check 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?

Since inspect.getmembers is an introspection function it shouldn't run any actual code (i.e. executing getters). It should compute its result statically. Using getattr_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 test test_getmembers_VirtualAttribute expects this method

@types.DynamicClassAttribute
    def eggs(self):
        return 'spam'

to be called. It asserts whether ('eggs', 'spam') is in inspect.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 how inspect.getmembers should behave.

Failing tests:

• test_get_slot_members
• test_getmembers_method
• test_getmembers_VirtualAttribute

Another inconsistent behaviour: inspect.isdatadescriptor should return True for properties:

Return true if the object is a data descriptor.

Data descriptors have both a get and a set method. Examples are properties (defined in Python), getsets, and members

(Python Docs)

However, the following code does not return any properties:

class Foo:
    def __init__(self, data):
        self._bar = data
    
    @property
    def bar(self):
        print('--BAR--')
        return self._bar

inspect.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', <property object at 0x...>)]

Is this correct so far? I am still confused about how getmembers should behave. See test_getmembers_VirtualAttribute, this test expects a different behaviour.

Additionally,

class Foo:
    pass

inspect.getattr_static(Foo('test'), '__class__')

returns <attribute '__class__' of 'object' objects> whereas getattr would return <class '__main__.Foo'>. What should getmembers return? If it returns <attribute '__class__' of 'object' objects>, the inspect.isclass predicate would not work:

class Foo:
    pass

# Will return [] if getattr_static is used
inspect.getmembers(Foo('test'), inspect.isclass)

In 3ac2093, I've changed inspect.getmembers to only use inspect.getattr_static for properties, as other members (e.g. __class__) are expected to be returned using getattr(obj,'__class__'):

getattr_static(obj,'__class__') returns <attribute '__class__' of 'object' objects> whereas getattr 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 phrase I 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, what getmembers 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 function getmembers_static: I still have some questions that should be discussed before doing so:

1. What should getmembers_static return? Is getattr_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 as getmembers. Would one just copy and paste the old ones? It would also share most of the code - except for maybe 1 line - with getmembers.

3. Is there a reason for getmembers to return a list of tuples? I would have suggested to use a dict 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.