Skip to content

Conversation

@jairideout
Copy link
Contributor

@jairideout jairideout commented Oct 31, 2018

ABC-derived classes (i.e. those deriving from the abc and collections.abc modules) now include their members (as defined in __dict__) in generated autosummary RST. __slots__ may still be used to override __dict__, but if an ABC-derived class has empty slots, its __dict__ will be used instead.

This was only an issue in Python 3 when :inherited-members: was not in effect.

Fixes #52

@astrofrog
Copy link
Member

@jairideout - thanks for the PR! It looks like tests are failing on Python 2.7 - since we are still Python 2 compatible (for now), could you see if there is an easy fix?

…thon 3

ABC-derived classes (i.e. those deriving from the `abc` and `collections.abc` modules) now include their members (as defined in `__dict__`) in generated `autosummary` RST. `__slots__` may still be used to override `__dict__`, but if an ABC-derived class has empty slots, its `__dict__` will be used instead.

This was only an issue in Python 3 when ``:inherited-members:`` was not in effect.

Fixes astropy#52
@jairideout jairideout changed the title Fix issue with ABC-derived classes having their members ignored Fix issue with ABC-derived classes having their members ignored in Python 3 Nov 1, 2018
@jairideout
Copy link
Contributor Author

Thanks for the heads up @astrofrog -- I amended my commit to add Python 2.7 support and the tests are now passing. I'm not sure what's going on with the drop in coverage (-0.9%). Let me know if that's something I should investigate further.

Note: while adding Python 2.7 support, I noticed that the bug was only an issue in Python 3 when :inherited-members: was not in effect. I updated the relevant docs/comments in the PR and added a note about this to the corresponding issue (#52).

Thanks for taking a look!

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants