Docstrings of "protected" members that are part of interfaces should be rendered
EliahKagan opened this issue · 0 comments
GitPython classes contain a number of conceptually "protected" methods that can reasonably be called by code in subclasses (including in subclasses' overridden implementations of them) but not by arbitrary other code.
By default, Sphinx autodoc does not render documentation for entities that start with _. That makes sense for conceptually "private" things that don't make sense for outside code to call and that could, without warning, change or be removed at any time. But for "protected" methods, in order to correctly use them, it is useful to have documentation.
As one example, the public git.objects.tree.Tree.traverse method has this as its docstring:
Lines 295 to 298 in fe1934c
Although that can (and should) be made into a :meth: reference to Traversable._traverse, doing so will not cause it to link to an rendered docstring for Traversable._traverse, because the _traverse method is non-public and there is currently nothing to link to for it in the rendered Sphinx documentation.
This could be mitigated in some specific cases by copying or moving documentation to fully public methods. For example, the Tree.traverse method could have all the relevant information present in the Traversable._traverse method. This would risk that information becoming out of date or otherwise confusing if it ends up being updated in one place but not another. But the real issue is that conceptually "protected" methods that document how they are and are not supposed to be used should--whether or not they are referenced in any other docstrings--be part of the rendered Sphinx documentation.
Sphinx autodoc generation is controlled by the code in reference.rst for each section of the reference manual. Currently most sections take the form:
GitPython/doc/source/reference.rst
Lines 48 to 51 in fe1934c
Sphinx autodoc supports :private-members:, which can be used to emit documentation for just some specifically named non-public members (rather than all) by giving them as arguments. But I suspect that this may not be the best way to do it. Each entity in GitPython that has specific documentation currently has it in the form of a docstring on that entity. So if possible, this should be controlled from the docstrings themselves. (Of course, if this is not feasible, then it is better to list them explicitly in reference.rst than to keep their documentation omitted forever.)
Fixing this may help with #1834, especially if the non-None usage to be documented is not going to be deprecated, since the recommendation to prefer returning None will probably have to be written in the docstring, and for this to really be clear the docstring should be rendered. However, I don't think this needs to be seen as a blocker for that, and I don't want to discourage anyone from fixing #1834 at any time.
Finally, note that sometimes the word "protected" is used in Python jargon to mean methods named like _foo rather than __foo. That's not what I mean; I am using this word in the traditional OOP sense to refer to members that are considered part of the interface for subclasses but not part of the interface more broadly. Most _foo methods in GitPython are conceptually private and should not have their docstrings emitted by Sphinx.