Fix docstrings in series.py #10712
Fix docstrings in series.py #10712
Conversation
vyasr
added
3 - Ready for Review
There was a problem hiding this comment.
Comments attached - please ignore anything that I suggested if it blatantly conflicts with pydocstyle rules that I did not consider. I caught myself at least once (forgetting that pydocstyle requires starting with a verb, for instance). On the whole, this is a nice change and will help us improve documentation throughout the library!
| @@ -3381,8 +3473,7 @@ def __init__(self, series): | |||
| @property # type: ignore | |||
| @_cudf_nvtx_annotate | |||
| def year(self): | |||
| """ | |||
| The year of the datetime. | |||
| """Get the year of the datetime. | |||
There was a problem hiding this comment.
pandas doesn't use the "Get" term here or on other dt properties.
| """Get the year of the datetime. | |
| """The year of the datetime. |
https://pandas.pydata.org/docs/reference/api/pandas.Series.dt.year.html
There was a problem hiding this comment.
I realize now that this is due to pydocstyle requiring a verb at the beginning. I wish it didn't require that for properties... oh well. Ignore this.
There was a problem hiding this comment.
Yeah this is an error code where I chose to prefer changing the docstring over adding noqas. If you change your mind and decide that you prefer the noqas let me know, I'm happy to change it back to that too.
| """ | ||
| Returns a DataFrame with the year, week, and day | ||
| calculated according to the ISO 8601 standard. | ||
| """Return a DataFrame with the year, week, and day calculated according to the ISO 8601 standard. |
There was a problem hiding this comment.
I'd wrap this (so it doesn't break E501) and instead choose to break whichever rule requires the brief to be one line, if that's equally viable.
There was a problem hiding this comment.
My preference is to consistently break E501 rather than selectively break any other rule. Using fewer distinct noqas makes it easier for developers, and it also reduces the degree of subjectivity (docstring A is really too long so let's do multi-line, whereas docstring B is just a tiny bit too long so let's allow E501 breakage). Breaking the one-line brief rule is also much more likely to lead to accidentally invalid docstrings if someone inserts an extra newline unintentionally.
Codecov Report
@@ Coverage Diff @@
## branch-22.06 #10712 +/- ##
================================================
+ Coverage 86.35% 86.39% +0.03%
================================================
Files 142 142
Lines 22335 22297 -38
================================================
- Hits 19287 19263 -24
+ Misses 3048 3034 -14
Continue to review full report at Codecov.
|
|
In light of our subsequent discussions on #10711, I'm going to close this PR since it makes a bunch of unnecessary and possibly undesirable changes. I'll reopen a new one with a minimal set of changes to demonstrate how we'd like these PRs to look going forward. |
This PR contributes to #10711 by fixing the style for series.py. It also ensures that pre-commit and running pydocstyle manually both produce the expected output so that future linting improvements (contributions to #10711) can be done as a good first issue by new contributors.