You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently _ImageBase is private, and not documented. This is an issue, because if I want to see what methods I can use with AxesImage, most of them are missing from the docs as most of them live in _ImageBase, which AxesImage inherits from.
This is a general problem of private base classes, and I've been reasoning about this before:
As written elsewhere, I have doubts that private base classes are a reasonable approach. Their code leaks into the child classes, which is intended, but OTOH confusing because you're not seeing the base class. This also messes up sphinx because the inherited methods are not documented and cannot be linked.
Generally, I cannot remember to have seen private base classes in other projects (though maybe they are well hidden by their nature). I feel it's better to make the base class public, but some of it's members private or abstract.
The only alternative would be a sphinx mechanism that allows to pull in documentation from parent classes. Which could reasonably work for .. class:: directives, but already hits limits for .. autosummary::. An option
Even more generally one has the problem of finding the supported methods of a class if it is in an object hirachy.
I think the Qt docs have a quite good solution. Additionally to the list of new or re-implemented members at the top of the class docs, they have linked a list of all members for each class. This would be really useful if somebody wants to write a sphinx plugin... 😄
Is there a good reason (in this case) for the class to be private? It looks like it has been private ever since it was introduced ~ 4yrs ago by @mdboom
Hmm, in that case I think I agree with @timhoffm when he says to make attributes and methods private, but make the actual class and some of it's methods public.
In this example, I was wanting to set the alpha value of an AxesImage, but that set_alpha is inside _ImageBase, so doesn't exist in the docs. Similarly for set_data, which I feel like is a pretty big documentation omission for the class returned by plt.imshow().
I don't think private classes should be made public only for the purposes of documentation. Is there any other reasons to make those public? If not, it's very likely there's a way to make the documentation public without without the class.
Why shouldn't the classes be made public? As long as all of the API that is part of the class (ie. methods, attributes, properties) that should be private is private, it seems sensible to me to expose the (in this case fairly crucial!) intended user facing API.
Classes that contain public methods are de facto public themselves. This doesn't prevent them from having additional privte attributes. So the change here totally makes sense to me.
It makes the base class public. The question is whether the base class should be public or not. The publicly available functions should still be available under anything that inherits from them.
As always, it's a question of maintenance burden long term.
I may be wrong here, but what is a class if not just a collection of methods and attributes? What else needs maintaining if we make the class itself public, apart from what is intended as user facing methods?
Let me expand a bit more… Once you make something public, it is an irreversible change. Thousands of people will start using anything that you make public and this is not something you can really revert. That means that those objects will now have to be fully tested, documented, and the API should never changed in a backward compatible manner.
Base classes should probably not be used directly by the users. As a result, they should not be exposed to the user. If the publicly available methods are not documented in the classes that inherit from these, it should be fixed in those classes instead of making the base class public.
In addition, the more publicly available functions and classes there are, the harder it is for the user to figure out which one to use. It's harder to use autocomplete in ipython (or other interactive interpreter). It's harder to figure out in the html documentation where to look.
There are many projects that have private "base" classes, some of which have excellent documentation. Maybe someone should check out how those are are documented?
Note that sklearn's typically duplicate a lot of the documentation to ease accessibility of contributions. You can also see that some of the BaseEstimators/Regressors are public: this is by design, as it is important for developers to be able to provide sklearn compatible estimators.
I'm sure I can find other examples: that's the examples I am most familiar with.
Just came upon this because I recommended a method to someone that they didn't realize was available! SciTools/cartopy#1543 (comment)
So, I think this would be nice to clear up in some way. I would have a slight preference for just pushing the inherited methods up publicly in the docs rather than making all the base methods public, but I don't have a strong preference either way.
Is there a reason we don't document all inherited members? Would it add a lot of extraneous bloat to the documentation in other places? Adding: 'inherited-members': None in the conf, would fix the private inheritance throughout the code.
@dstansby, it looks like you went a step further and added some extra things in your closed PR. I'm wondering if I'm missing something basic in just adding the inherited-members to the rst file?
Activity
timhoffm commentedon Oct 24, 2019
This is a general problem of private base classes, and I've been reasoning about this before:
The only alternative would be a sphinx mechanism that allows to pull in documentation from parent classes. Which could reasonably work for
.. class::directives, but already hits limits for.. autosummary::. An optionwould be quite awkward.
Even more generally one has the problem of finding the supported methods of a class if it is in an object hirachy.
I think the Qt docs have a quite good solution. Additionally to the list of new or re-implemented members at the top of the class docs, they have linked a list of all members for each class. This would be really useful if somebody wants to write a sphinx plugin... 😄
dstansby commentedon Oct 25, 2019
Is there a good reason (in this case) for the class to be private? It looks like it has been private ever since it was introduced ~ 4yrs ago by @mdboom
tacaswell commentedon Oct 25, 2019
My memory of why is because on it's own it is not usable as an artist, but holds a bunch of shared code for sub-classes (either existing or planned).
There is also the general concern about adding public API (as once we do, it is very hard to take back).
dstansby commentedon Oct 26, 2019
Hmm, in that case I think I agree with @timhoffm when he says to make attributes and methods private, but make the actual class and some of it's methods public.
In this example, I was wanting to set the alpha value of an
AxesImage, but thatset_alphais inside_ImageBase, so doesn't exist in the docs. Similarly forset_data, which I feel like is a pretty big documentation omission for the class returned byplt.imshow().tacaswell commentedon Oct 26, 2019
We have also on exercised the ability to do anything with the private class in the 4 years we have had it.
On a bit more consideration I am 👍 on making all of our private base-classes public.
NelleV commentedon Nov 27, 2019
I don't think private classes should be made public only for the purposes of documentation. Is there any other reasons to make those public? If not, it's very likely there's a way to make the documentation public without without the class.
dstansby commentedon Nov 27, 2019
Why shouldn't the classes be made public? As long as all of the API that is part of the class (ie. methods, attributes, properties) that should be private is private, it seems sensible to me to expose the (in this case fairly crucial!) intended user facing API.
ImportanceOfBeingErnest commentedon Nov 27, 2019
Classes that contain public methods are de facto public themselves. This doesn't prevent them from having additional privte attributes. So the change here totally makes sense to me.
NelleV commentedon Nov 27, 2019
It makes the base class public. The question is whether the base class should be public or not. The publicly available functions should still be available under anything that inherits from them.
As always, it's a question of maintenance burden long term.
dstansby commentedon Nov 27, 2019
I may be wrong here, but what is a class if not just a collection of methods and attributes? What else needs maintaining if we make the class itself public, apart from what is intended as user facing methods?
NelleV commentedon Nov 27, 2019
Let me expand a bit more… Once you make something public, it is an irreversible change. Thousands of people will start using anything that you make public and this is not something you can really revert. That means that those objects will now have to be fully tested, documented, and the API should never changed in a backward compatible manner.
Base classes should probably not be used directly by the users. As a result, they should not be exposed to the user. If the publicly available methods are not documented in the classes that inherit from these, it should be fixed in those classes instead of making the base class public.
In addition, the more publicly available functions and classes there are, the harder it is for the user to figure out which one to use. It's harder to use autocomplete in ipython (or other interactive interpreter). It's harder to figure out in the html documentation where to look.
There are many projects that have private "base" classes, some of which have excellent documentation. Maybe someone should check out how those are are documented?
anntzer commentedon Nov 27, 2019
Can you provide an example?
NelleV commentedon Nov 27, 2019
Sure, but it turns out it's more tricky than being private using an underscore :)
https://scikit-learn.org/stable/modules/classes.html#module-sklearn.cluster.bicluster
The biclustering algorithms "SpectralBiclustering" and "SpectralCoclustering" (capitalization may be wrong) inherent from SpectralBase which you can not easily find, neither in the documentation nor through ipython's import.
Note that sklearn's typically duplicate a lot of the documentation to ease accessibility of contributions. You can also see that some of the BaseEstimators/Regressors are public: this is by design, as it is important for developers to be able to provide sklearn compatible estimators.
I'm sure I can find other examples: that's the examples I am most familiar with.
6 remaining items
greglucas commentedon May 9, 2020
Just came upon this because I recommended a method to someone that they didn't realize was available! SciTools/cartopy#1543 (comment)
So, I think this would be nice to clear up in some way. I would have a slight preference for just pushing the inherited methods up publicly in the docs rather than making all the base methods public, but I don't have a strong preference either way.
greglucas commentedon May 13, 2020
Is there a reason we don't document all inherited members? Would it add a lot of extraneous bloat to the documentation in other places? Adding:
'inherited-members': Nonein the conf, would fix the private inheritance throughout the code.matplotlib/doc/conf.py
Line 104 in a6f82b9
While we could just fix this specific instance by adding
:inherited-members:to the image_api documentationhttps://github.com/matplotlib/matplotlib/blob/master/doc/api/image_api.rst
which is already done in the collections documentation:
https://github.com/matplotlib/matplotlib/blob/master/doc/api/collections_api.rst
@dstansby, it looks like you went a step further and added some extra things in your closed PR. I'm wondering if I'm missing something basic in just adding the inherited-members to the rst file?
timhoffm commentedon May 13, 2020
Did not check, but I assume we would flood the documentation with artist methods if we would globally inherit docstrings (https://matplotlib.org/devdocs/api/artist_api.html).