python / cpython

The Python programming language
https://www.python.org
Other
63.69k stars 30.51k forks source link

Have inspect.getdoc follow the MRO when a method docstring is only one line. #94470

Open gpshead opened 2 years ago

gpshead commented 2 years ago

Feature or enhancement

"Hot" on the heels of 3.5ish's https://github.com/python/cpython/issues/59787 comes another idea: An age old pattern we have in our codebase is for people to put """See base class.""" as the docstring on methods to appease lint tooling demanding a docstring on non-trivial methods. As a consequence code written this way doesn't trigger the inspect.getdoc() feature from #59787 and the help() text in a Notebook for such functions is the content-free oneliner instead of heading up the MRO chain.

I propose a new feature: If the docstring for a method exists but is <= 1 line, follow the MRO as if there were no docstring and return

f"{onelinedoc}:\n{mro_doc}" as the docstring. Do this recursively, but collapse duplicate docstrings gathered along the way so that you could wind up with [good]:

"""See base class:
Fill a hovercraft with eels of the chosen species.

Args:
  species: A set of allowed Eel types.
  count: float, a non-negative number of eels to add. Fractions are allowed.
"""

rather than this degenerate pattern in deep heirarchies that could otherwise result [bad]:

"""See base class:
See base class:
See base class:
Fill a hovercraft with ...
"""

Collapsing identical docstrings along the heirarchy in this case also works for the situation where they're all the same cut and pasted oneliner and nothing else.

stevendaprano commented 2 years ago

I don't think that it is a good idea to build a heuristic into help() to overcome a problem caused by a dubious fix for an overzealous linter.

Instead of polluting your methods with a low-information message to satisfy an overzealous linter, why not disable that check? Or report it as a bug to the linter and get them to fix the problem at the source.

This heuristic can do the wrong thing.

class HovercraftSansEels(Hovercraft):
    def fill(self, *args):
        """Override the superclass method and prevent the hovercraft from being filled with eels."
        return None

which would give us the following help text:

"""Override the superclass method and prevent the hovercraft from being filled with eels:
Fill a hovercraft with eels of the chosen species.

Args:
  species: A set of allowed Eel types.
  count: float, a non-negative number of eels to add. Fractions are allowed.
"""
gpshead commented 2 years ago

Ignore the scenario I listed and realize that human nature can cause this doc pattern as well. It's a wart in all OO languages: how does one document overridden methods and where?

Even if getdoc() doesn't do MRO following by default triggered via such an potentially arbitrary doc contents heuristic, I do think there's value in having an easy way to do it to provide information to the user. To alleviate confusion about the fate of the Eels, have this generate a version with the class name in the result.

stevendaprano commented 2 years ago

Relevant: #94506

I hear what you are saying, but I am wary about overloading interactive help with even more excessive detail and duplicated text. Imagine, if you will, being faced with help text:

class Child(Parent)
 |  Methods defined here:
 |  
 |  verbify(self)
 |      See base class
 |      Long docstring ...
 |      that extends over ...
 |      very ...
 |      many ...
 |      lines.

class Parent(builtins.object)
 |  Methods defined here:
 |  
 |  verbify(self)
 |      Long docstring ...
 |      that extends over ...
 |      very ...
 |      many ...
 |      lines.

Duplicating content like that is not good for the reader.

help(big_module) is already overwhelming. help() of a big module with many subclasses, each with one-line docstrings in overridden methods, would become positively agonizing if we started duplicating the docstrings of methods.

Even in the case where you refer directly to a method, using help(Child.verbify), I don't think it is a large imposition on the user (i.e. me) to read the text "See base class", identify the base class, and then run help(Parent.verbify). I prefer that to dealing with overwhelming amounts of duplicated text.

I do agree with you that help ought to do a better job of identifying where methods are defined, to streamline that identification.

gpshead commented 2 years ago

Heh, this feature request is turning more into a treatise on what makes a useful help system. 😛

Perhaps inspect.getdoc should remain conservative (fair), but I think we've got room for something more generally meaningful than what it does.

I'd want to take a look at existing solutions for this first before proposing a new smarter heuristic doc generation API. Notebooks and others may have created something helpful here already?