DOC: Capitalize docstring summaries #23886
Conversation
| Hello @varadgunjal! Thanks for updating the PR.
Comment last updated on November 24, 2018 at 21:44 Hours UTC |
Codecov Report
@@ Coverage Diff @@ ## master #23886 +/- ## ======================================= Coverage 92.31% 92.31% ======================================= Files 161 161 Lines 51483 51483 ======================================= Hits 47525 47525 Misses 3958 3958
Continue to review full report at Codecov.
|
datapythonista changed the title 
datapythonista left a comment
datapythonista left a comment There was a problem hiding this comment.
Looks great, thanks for working on this. I added some comments with things that besides the capitalization are wrong in the format. If you don't mind taking care of those too, that would be really great.
| @datapythonista I've been diving into the code further to see where I can contribute and have noticed that a lot of these issues are repeated in other files that do not show up when the validate script runs. The changes will affect a lot of files - including editing/updating some docstrings. Shall go ahead and make the changes there and submit PRs module-by-module? Or should I just stick to the files that the script found for this issue? |
| I think the script should like all the docstrings that do not start with a capital letter in the public API. I'd leave just those in this PR. Fixing the others would be nice, but is not as important, as the docstrings are not in the pandas documentation web. If you take care of them, do it in chunks, huge pull requests make reviews complicated and not very efficient. But there should be issues labeled as "good first issue" that are probably more important at this point. |
datapythonista left a comment
datapythonista left a comment There was a problem hiding this comment.
Really nice changes, thanks @varadgunjal.
Added some comments, if you can take care of them, and I'll try to get this merged soon, before it causes conflicts (being so big it's likely to happen). Not so important, but for the next time, it usually makes things easier and more efficient if you split a PR of this size in two or three chunks.
pandas/core/indexes/base.py Outdated
| """ | ||
| Alias for is_monotonic_increasing. | ||
| .. deprecated :: """ |
There was a problem hiding this comment.
This is not actually deprecated (see that the code doesn't raise a deprecation warning.
Can you remove the deprecated directive (also the deprecated in brackets as you did).
@jreback do we want to deprecate this? (that would be for another PR)
pandas/core/indexes/base.py Outdated
| def _is_strictly_monotonic_increasing(self): | ||
| """return if the index is strictly monotonic increasing | ||
| (only increasing) values | ||
| """Return if the index is strictly monotonic increasing |
There was a problem hiding this comment.
Can you move this one to the next line of the quotes too?
| """ | ||
| where : array of timestamps | ||
| mask : array of booleans where data is not NA | ||
| Parameters |
| Sorry that the PR has become quite big. I've stuck to fixing issues in the files reported by the |
datapythonista left a comment
datapythonista left a comment There was a problem hiding this comment.
Just one thing that I'm not sure is correct, other than that looks great.
datapythonista left a comment
datapythonista left a comment There was a problem hiding this comment.
Looks great @varadgunjal, thanks a lot for all the fixes.
@pandas-dev/pandas-core can someone take a look at this? It's just fixes to the docstrings formats, but it's plenty of them, and would be great to get this merged before it starts to have conflicts.
| can you merge master |
| Thanks @varadgunjal for all the work with this, and thanks @gfyoung for the review. |
5 participants
git diff upstream/master -u -- "*.py" | flake8 --diffvalidate_docstrings.pyscript.The few issues that still show up on running the script can be attributed to-
tseries.offsets.Tick,tseries.offsets.Dayetc. (working on this for a separate PR)Series.argmin,Timestamp.strptimeetc.