Skip to content
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

choose a docstring convention #12074

Open
danieleades opened this issue Mar 13, 2024 · 11 comments
Open

choose a docstring convention #12074

danieleades opened this issue Mar 13, 2024 · 11 comments
Labels

Comments

@danieleades
Copy link
Contributor

danieleades commented Mar 13, 2024

ruff can be used to enforce 3 different docstring conventions.

options are google, numpy, and pep257

We should pick one, configure Ruff to lint for it, and fix/whitelist any existing errors

see https://docs.astral.sh/ruff/settings/#lint_pydocstyle_convention

@danieleades danieleades added the type:proposal a feature suggestion label Mar 13, 2024
@electric-coder
Copy link

Choose Google docstring style! It's the most readable, concise, etc...

@danieleades
Copy link
Contributor Author

Choose Google docstring style! It's the most readable, concise, etc...

that would likely be my preference too

@chrisjsewell
Copy link
Member

chrisjsewell commented Mar 13, 2024

options are google, numpy, and pep257

the sphinx codebase already uses the standard sphinx implementation for docstrings, not google or numpy, therefore we should only use pep257

@picnixz
Copy link
Member

picnixz commented Mar 14, 2024

Personally, I hate Google/NumPy docstrings although they are clearer, so let's go for pep257 (also it's good to stick to PEPs if possible).

@electric-coder
Copy link

let's go for pep257 (also it's good to stick to PEPs if possible)

The other 2 styles are actually also compliant with the PEP specification. But since this FR is more about activating a linter option than changing the codebase the simpler option of keeping the docstrings as they are currently is the easier choice - so lets go with that.

@danieleades
Copy link
Contributor Author

just to be clear, the main immediate impact will be a simplification of the ruff config.

the 'pydocstyle' lint group (D) has a number of conflicting configuration options corresponding to different conventions. Right now these are manually toggled on or off. Choosing a convention automatically toggles the appropriate rules on and off to match the convention.

the pydocstyle lints also enforce documentation of public objects. These rules are currently suppressed, and would likely remain suppressed (at least for now).

Sphinx doesn't appear to me to have a clear distinction between public and private interfaces, but that's a problem for another day

@picnixz
Copy link
Member

picnixz commented Mar 15, 2024

I like the idea of getting rid of as many config option we can. Concerning the public API, we usually document what is really public at the rst level as well so I think we'll not enforce the public docstrings.

As for the private/public, we need to make it clear in another issue as you said.

@danieleades
Copy link
Contributor Author

I like the idea of getting rid of as many config option we can. Concerning the public API, we usually document what is really public at the rst level as well so I think we'll not enforce the public docstrings.

As for the private/public, we need to make it clear in another issue as you said.

the public api documentation lints respect both conventions for defining public objects. Namely, using leading underscores to denote private objects and modules, and using __all__ statements to explicitly list public objects. I personally prefer using __all__, and reserving the leading underscore to objects which are module-private (that is, not expected to be imported even from within other modules in the same package)

@picnixz
Copy link
Member

picnixz commented Mar 15, 2024

Namely, using leading underscores to denote private objects and modules

In general yes, but there are objects that are "publicly named" but were not meant (originally) to be part of the public API. A good example for that one would be the ModuleAnalyzer which is not documented and IMO an implementation detail.

and using all statements to explicitly list public objects

It would be good as a convention indeed, but currently Sphinx uses it only for a few modules (probably because of who actually wrote the code at that time).

reserving the leading underscore to objects which are module-private

In general, that's what I would expect as well but ... it's not the case everywhere I think.

Now, before going off-topic, let's open another issue (could you?) concerning public/private API and how we should go from now on.

@AA-Turner
Copy link
Member

Agree on pep257, save in e.g. ext.napoleon.

A

@danieleades
Copy link
Contributor Author

Now, before going off-topic, let's open another issue (could you?) concerning public/private API and how we should go from now on.

see #12246

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

Successfully merging a pull request may close this issue.

5 participants