Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autosummary additional links for inherited methods and attributes #11712
base: master
Are you sure you want to change the base?
Autosummary additional links for inherited methods and attributes #11712
Changes from 4 commits
e62a37c
4e71d5b
6a2b58a
2f4d125
bee1d6b
4302a97
092f706
063d33f
b8dff3f
697be33
76c0194
192ad95
e9edee7
b7a0049
0901005
de412e0
4f3aa01
8e62aed
71c1729
89adbd9
2d56014
bff342f
13895a9
7f06fcb
8ea3cba
219cc23
3199fe0
9884579
5421b26
a34b648
6f3fbc3
9037a22
224076b
065a1a0
58ff1e3
3c9b0bb
336d3e3
ceb2b99
479ff7d
4987598
a3adf19
d6abcbe
73827e2
242f21a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it's better to have use
ns['all_methods']
since otherwise you'll only include public ones.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@satr-cowi Have you addressed this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my preference is for ns['inherited_methods'] would be to only return the public methods (and therefore be consistent with ns['methods'])
We could introduce ns['all_inherited_methods'] as well, but the documentation doesn't include anything about ns['all_methods'] at the moment so we should probably include that too if we do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I want us to be consistent without breaking previous changes.
AFAIU the current behaviour is "ns[C] returns public members for category C" and "ns[all_C] returns all members for category C (including private ones)". I don't mind not documenting "all_*" for now, but I would like the documentation for ns["members"] & co. to be updated then. Currently it says (for "members"):
In addition, your "inherited_qualnames" documentation tells me about:
So now your members also include private members.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current behaviour (as in the example) is:
I've updated the "members" documentation to make it clear that this and "inherited members" do include private members. It was already clear that methods/attributes were only public.
The proposed additions are just that:
Think this is pretty clear from the documentation now