[FEA] Lint all Python docstrings using ruff's pydocstyle rules

[vyasr]

Is your feature request related to a problem? Please describe.
Our Python docstrings have various style violations when compared against standards like pep257. Not only does this impact readability (which may be subjective), it also reduces the effectiveness of tools like Sphinx or numpydoc that rely on specific formatting in order to parse docstrings.

Describe the solution you'd like
We should lint all of our docstrings using pydocstyle, the standard for docstring linting in the Python ecosystem. I added support for pydocstyle in #7985 and have since added support for a few additional modules, but many of our most prominent public-facing APIs are not yet linted. My reticence to lint those files has stemmed in large part from the degree to which these files have been refactored recently and the number of docstrings that were rendered obsolete in the process. However, that process is largely complete now, at least for the most important public APIs, so I think we are at a good stage to start working through the remaining changes systematically.

While we have added pydocstyle on a per-file basis up to now, not all of pydocstyle's rules make sense for us since matching pandas is a higher priority. As a result, going forward we will enable rules on a per-rule rather than per-file basis so that we can get the most value out of pydocstyle without conflicting with preexisting pandas conventions. For more extensive discussion of different rules and why we choose to enable some or not, see the excellent table compiled by @bdice a few comments below..

[shwina]

Looking at the changes in #10712, I'm not convinced that many of them are improvements. Can we take a step back and evaluate how exactly pydocstyle is going to help us concretely?

it also reduces the effectiveness of tools like Sphinx or numpydoc that rely on specific formatting in order to parse docstrings.

Do we have examples where this doesn't work well today?

Can we configure pydocstyle to catch a more narrow class of errors for us and ignore others?

[vyasr]

I agree with you. I have generally been a fan of docstring linting with pydocstyle in the past, but this PR definitely made me question its usability for cudf a little bit as well. Fundamentally I think that we have one major difference in this project relative to other projects that makes some of these changes feel less useful: we want to match pandas documentation as much as possible. As a result a lot of these changes may end up working contrary to our goals. We may end up getting better mileage by writing a custom tool that performs an element-wise comparison of docstrings of all public APIs against their pandas equivalents and highlights differences.

Looking at the changes in #10712, I'm not convinced that many of them are improvements. Can we take a step back and evaluate how exactly pydocstyle is going to help us concretely?

it also reduces the effectiveness of tools like Sphinx or numpydoc that rely on specific formatting in order to parse docstrings.

Do we have examples where this doesn't work well today?

I think the main things that pydocstyle catches that help us are errors that lead to improper Sphinx HTML generation. For instance, in this PR I fixed places where there are newlines in places where they should not be present in between section headers like "Parameters" and their underlines. There were also a few places where the summary line was not separated from subsequent lines. That can also lead to improper Sphinx.

More generally, pydocstyle can be helpful to catch missing docstrings, missing parameters, missing types, etc, but in general I think we've done OK with those because we copy so many of our docs from pandas. We're benefiting less from the tool because we're not writing docstrings from scratch.

Can we configure pydocstyle to catch a more narrow class of errors for us and ignore others?

We can choose to only lint a specific set of pydocstyle's supported error codes. However, I question how useful it will end up being since pandas probably has minor violations of pydocstyle style all over the place. If we block too many of the rules, the tool's value will decrease substantially.

CC @bdice, who I'm sure will have more opinions here 😄

[vyasr]

To clarify my comment from above and reiterate some of my offline comments to @shwina:

I am personally in favor of using opinionated linters like pydocstyle. Even in cases where they disagree with my personal preferences, I prefer using them because they reduce the number of discussions that need to be had over what are usually very minor, very subjective issues. I think there is a path for us to get the best of both worlds here where we use pydocstyle but specify a limited set of error codes to check, then add an additional tool to do the pandas comparison at a later point. Disabling more of pydocstyle's checks in favor of pandas docstring choices may be our best options, as long as we can do so consistently.

[bdice]

I think the main things that pydocstyle catches that help us are errors that lead to improper Sphinx HTML generation. For instance, in this PR I fixed places where there are newlines in places where they should not be present in between section headers like "Parameters" and their underlines. There were also a few places where the summary line was not separated from subsequent lines. That can also lead to improper Sphinx.

We can choose to only lint a specific set of pydocstyle's supported error codes.

This is the direction I would like to see us go. Rather than enforce all of pydocstyle's rules on specific files, we should use it very broadly to fix a select set of minor issues that result in invalid Sphinx. Otherwise we should align 100% with pandas' wording wherever we can, instead of adhere to pydocstyle's grammatical preferences.

Here's an overview of my thoughts on the error codes to enable. Any rule that causes significant friction should be scrutinized more carefully.

• Missing Docstrings D100-D107. ❌ Skip all of these because they can't recognize inheritance or non-static docstrings injected at runtime. Missing docstrings are sometimes better than poorly formatted / badly rendering docstrings.
• Whitespace Issues D200-D215. ✔️ This category is useful. I would enable whatever subset of these is most consistent with our intended NumPy-style docstrings. NumPy does not enable D203, D212, D213. Perhaps other exceptions are needed as well, I'm not sure.
• Quotes Issues D300-D302. ✔️ I would enable all of these.
• Docstring Content Issues D400-D418. Enable only a select few. Perhaps the following:

Enable?	Code	Description	Notes
✔️	D400	First line should end with a period
❌	D401	First line should be in imperative mood
❌	D401	First line should be in imperative mood; try rephrasing
❌	D402	First line should not be the function’s “signature”	Appears to conflict with using parentheses, e.g. mode(s), and disabled by default for numpy conventions
✔️	D403	First word of the first line should be properly capitalized
❌	D404	First word of the docstring should not be This	Seems arbitrary -- might conflict with pandas
✔️	D405	Section name should be properly capitalized
✔️	D406	Section name should end with a newline
✔️	D407	Missing dashed underline after section
✔️	D408	Section underline should be in the line following the section’s name
✔️	D409	Section underline should match the length of its name
✔️	D410	Missing blank line after section	Not sure if we need both this and D411?
✔️	D411	Missing blank line before section	Not sure if we need both this and D410?
✔️	D412	No blank lines allowed between a section header and its content
❌	D413	Missing blank line after last section	Disabled by default for numpy conventions
✔️	D414	Section has no content
❌	D415	First line should end with a period, question mark, or exclamation point	Disabled by default for numpy conventions
❌	D416	Section name should end with a colon	Disabled by default for numpy conventions
❌	D417	Missing argument descriptions in the docstring	Disabled by default for numpy conventions
✔️	D418	Function/ Method decorated with @overload shouldn’t contain a docstring

[vyasr]

Regarding missing docstrings, I agree in part. I have two main modifications to Bradley's comments above:

• I think D102 is the only one that we'll actually need to disable because a) only methods are affected by inheritance, and b) methods are by far the most common use case for runtime injected docstrings. There may be a few violations of D103, especially in cuIO, but overall I think free functions will typically have docstrings written and I would rather manually noqa the few cases where we do need to skip D103. (We should of course disable D107 since we document that in the class docstring rather than the method)
• When we initially apply D102 to a file (when it is added to the match in python/.flake8) D102 should be enabled. The reason is that there are public-facing methods that are actually missing documentation and are not inheriting docstrings (not many, but they do exist). Maybe we just need to document that in this issue so that developers are aware that they need to check this.

[shwina]

Agree with the excellent analysis from you both!