-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Try documenting inherited class members #15780
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
Conversation
AFAICT this doesn't help... https://27030-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/api/image_api.html. |
That worked 👍 but now there are issues with cross-references becoming ambiguous :/ |
Before I embark on fixing the ambiguous cross references, do we think this is the right way to go? (cc @story645) |
I think this is the right way to go 👍 |
The approach is fine, although hopefully the references fix won't involve replacing each instance of .Axes.foo by .axes.Axes.foo... (I think this would be annoying wrt the readability of non-rendered docstrings too -- possibly you can get away with making the other Axes classes in mpl_toolkits not show inherited members?) |
I did not think of this, thanks for suggesting it before I tried to replace everything!! |
Or even hiding them from the docs, I'd rather slightly worsen the docs for mpl_toolkits rather than making the rest of the docs worse. |
Don't quite know where to document, but no longer trying to reserve cross reference issues for a sprint so they're fair game for people to fix. I'm with @NelleV that I think this is a good alternative than making a lot of private classes public. How much worse would the the mpl_toolkits docs be @anntzer ? |
Dunno, I guess that likely involves not documenting the mpl_toolkits variants of Axes in autogenerated docs? (which would just be mostly duplicating the standard Axes anyways...) They can still be documented in plain text. |
Oh, it should be fine to configure inherited-members to be False in the toolkits so they just look the same as before this change to the rest of the docs (I just haven't got round to trying it yet). |
Did not test, but if we globally enable One way could be to set |
Thinking about it I quite like having all the inherited methods available for each class, avoiding the need to click through the class inheritances to discover all the methods I can use for a particular class. |
In general I agree with this, but I think mpl_toolkits Axes is a special enough case (it's basically the same as normal Axes but with different positioning methods; I would guess what most people care about is the positioning, not the (same) plotting methods). |
Generally, I agree. However as of now there is no index to the methods of the class, which leaves you with scrolling through an endless list to search the few relevant methods among all the boilerplate. To keep it usable, we'd need at least some autosummary (I think #11572 moves towards that direction, but didn't check in detail anymore). Even better would be something along the lines of the Qt documentation, which lists the new methods in a class, but has a link to an all-members list (e.g. https://doc.qt.io/qt-5/qpushbutton.html). But I don't think something like this exists for sphinx. |
1c7442e
to
366518b
Compare
Quick update: I think I have this working now excluding inherited |
Unless someone wants to pay me to do it, I don't going through this route and fixing everything is the way to go. I'll try and go the other way and enable the inheritance on a class by class basis where there's problems. |
Out of curiosity, the remaining problems are due to docstrings ill-formatted or is there something deeper? |
A combination of ill-formatting, and also lots of duplicate documentation (ie. if multiple items inherit from |
Yet, I feel it's the right approach. Would doing the inheritance class by class basis and then switch to the proposed solution in this PR be an option? |
Yes, I definitely think that's a way to go - hopefully squashing out all of the problems module by module will be feasible as opposed to doing in one go (which was this attempt) |
This should solve documentation issues where a private base class contains methods etc. that are hidden (because the class is private), but should be exposed in any class that inherits from the base class.