Skip to content

Conversation

@meshde
Copy link
Contributor

@meshde meshde commented Jan 11, 2020

@googlebot googlebot added the cla: yes Author has signed CLA label Jan 11, 2020
@dbieber
Copy link
Collaborator

dbieber commented Jan 13, 2020

Thanks for the PR!
LGTM at a first read.
Looks like the remaining travis failure is unrelated to your PR and you can ignore it.

@dbieber
Copy link
Collaborator

dbieber commented Jan 13, 2020

For reference, here is the definition of getmembers:

https://github.com/python/cpython/blob/9dbf5d3bc2033940cdca35440cf08814544f81e4/Lib/inspect.py#L316-358.

It's gotten more complicated than it used to be in order to handle some edge cases.

@dbieber dbieber merged commit 1ac5105 into google:master Feb 21, 2020
@dbieber
Copy link
Collaborator

dbieber commented Feb 21, 2020

Thanks for the contribution!

@dbieber
Copy link
Collaborator

dbieber commented Feb 21, 2020

In addition to fixing #142, it also prevents properties from being evaluated unnecessarily. This prevents unintended side-effects (if evaluating a property has a side-effect) and fixes the case where evaluating a property would have raised an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Author has signed CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using __getattr__ breaks python-fire

3 participants