Skip to content

Put constructor parameter docstring into __init__ docstrings #4414

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

Open
Tracked by #7053
michaelosthege opened this issue Jan 8, 2021 · 15 comments
Open
Tracked by #7053

Put constructor parameter docstring into __init__ docstrings #4414

michaelosthege opened this issue Jan 8, 2021 · 15 comments

Comments

@michaelosthege
Copy link
Member

Description of your problem

Reviewing #4411 I noticed that many classes have the documentation about the __init__ parameters in the class docstring and are missing the __init__ docstring entirely.

According to PEP 257...

The docstring for a class should summarize its behavior and list the public methods and instance variables. If the class is intended to be subclassed, and has an additional interface for subclasses, this interface should be listed separately (in the docstring). The class constructor should be documented in the docstring for its __init__ method.

(formatting by me)

Examples:

@ashish-hacker
Copy link
Contributor

ashish-hacker commented Jan 13, 2021

I understand the usefulness docstrings at right places. Can I take this responsibility to place doctrings about the parameters at the right place?
My suggestion ->

Before

class Example:
""" ....
S = .....
...."""
def __init__(self, S,...):
// code

After

class Example:
""" ......
......"""
def __init__(self, S,...):
""" S = ....
......"""
// code

@michaelosthege
Copy link
Member Author

@ashish-hacker wait. I was about to say yes and refer you to the numpydoc guide.
In contrast to PEP 257, the numpydoc guide actually instructs to put the parameters into the class docstring: https://numpydoc.readthedocs.io/en/latest/format.html#class-docstring

Pinging @ColCarroll for his opinion on the question: Should we follow PEP 257 (this issue) or numpydoc (close this issue)?

@ashish-hacker
Copy link
Contributor

Thanks @michaelosthege for the clarification.
I saw the code , and I think the docs are according to numpydoc standards and syntax. But If PEP 257 will be followed , I am eager to contribute.

@ColCarroll
Copy link
Member

Sorry for missing this! tl;dr: I'm strongly in favor of this.

Given the number of people using pymc3 in jupyter notebooks with tab completion, I have a strong desire to putting parameters in __init__, even if the rest of the docstring is just """Construct MetropolisHastings.\n...""".

The numpydoc guidance strikes me as funny to the point of maybe being a typo -- classes are sometimes intended to be constructed from other entry points than init (or in addition to init). For example, I can not create a pd.DataFrame other than through a classmethod without consulting the docstring of DataFrame.init.

Will numpydoc flag this?

@OriolAbril
Copy link
Member

I personally have a slight preference for numpydoc style and documenting classes and their init method in the class docstring, but any of the two options are fine.

To help make an informed decision, I have checked how html and jupyter docs are generated, and it looks like what is shown is first the class docstring followed by the init docstring (and in cases where the init docstring is empty, the init of the parent is pulled in). Here is the output for Metropolis:

html

image

jupyter

image
image

Will numpydoc flag this?

Haven't checked, but I bet that as long as there is a class docstring (even if only one line) numpydoc won't complain.

@michaelosthege
Copy link
Member Author

In VSCode the class docstring is shown while typing the class name, but after opening the brackets for the constructor it shows just the (in our case empty) __init__ docstring:
grafik

Therefore I'm also strongly in favor of following PEP 257.

@OriolAbril
Copy link
Member

What happens with pandas.DataFrame in VSCode? This is purely curiosity, I have no problem with documenting things in the init method, in fact, IIRC, there is a place somewhere in numpydoc style where it says that you should follow PEP 257 and that numpydoc provides extra and specific guidance to build on that. Let me see if I can find it.

@OriolAbril
Copy link
Member

It's in pandas docstring guide instead of numpydoc ones: https://pandas.pydata.org/pandas-docs/stable/development/contributing_docstring.html right below the first code block. 🤷

@michaelosthege
Copy link
Member Author

michaelosthege commented Jan 29, 2021

@OriolAbril in VSCode the DataFrame __init__ docstring is not shown.

grafik

grafik

@ashish-hacker
Copy link
Contributor

ashish-hacker commented Jan 29, 2021

Screenshot from 2021-01-29 14-58-12
Just leaving the screenshot of what @OriolAbril wanted to share I guess.( from https://pandas.pydata.org/pandas-docs/stable/development/contributing_docstring.html)
So, Will it be a good idea to follow PEP 257?

@ashish-hacker
Copy link
Contributor

So, Now Can I work on this (by following PEP 257)? @michaelosthege

@michaelosthege
Copy link
Member Author

@ashish-hacker yes, go ahead

@michaelosthege
Copy link
Member Author

michaelosthege commented Jan 29, 2021

@ashish-hacker and make sure to start out with a smaller PR - maybe just the samplers first. Just so we can see if it causes problems with the documentation

@ashish-hacker
Copy link
Contributor

Sure @michaelosthege . I will do it for a single class first or a file. Then You can review it.

@ashish-hacker
Copy link
Contributor

ashish-hacker commented Feb 1, 2021

Can you guide me @michaelosthege ? Will I continue doing this for other classes in the code base?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants