DOC: Fix Series nsmallest and nlargest docstring/doctests #22731
Conversation
|
Hello @Moisan! Thanks for updating the PR.
Comment last updated on September 18, 2018 at 01:46 Hours UTC |
| - ``first`` : take the first occurrences based on the index order | ||
| - ``last`` : take the last occurrences based on the index order | ||
| - ``all`` : keep all occurrences. This can result in a Series of | ||
| size larger than `n`. |
There was a problem hiding this comment.
Is the period here on the last bullet required to pass the docstring validation as-is? Shouldn't be necessary but if that's the intent here just something we should address separately @datapythonista
There was a problem hiding this comment.
I confirm that the validation fails if the last period is not present.
pandas/core/series.py
Outdated
| Brunei 434000 | ||
| dtype: int64 | ||
|
|
||
| >>> s.nlargest(3) |
There was a problem hiding this comment.
I think it would be nice to have just a quick one-liner to highlight the difference between this and the subsequent example
There was a problem hiding this comment.
Do you mean a comment at the end of the line?
There was a problem hiding this comment.
No just some text in between the examples to call out what the user should be looking at
pandas/core/series.py
Outdated
| Brunei 434000 | ||
| dtype: int64 | ||
|
|
||
| The n largest elements where n=3 with all duplicates kept. Note that the |
There was a problem hiding this comment.
Looks like this line is too long: https://travis-ci.org/pandas-dev/pandas/jobs/429733867#L3699
pandas/core/series.py
Outdated
| Monserat 5200 | ||
| dtype: int64 | ||
|
|
||
| The n largest elements where n=5 by default. |
There was a problem hiding this comment.
single backticks around n. Double backticks around n=5.
pandas/core/series.py
Outdated
| Brunei 434000 | ||
| dtype: int64 | ||
|
|
||
| The n largest elements where n=3. Default keep value is 'first' so |
There was a problem hiding this comment.
Same. Single backticks around keep.
pandas/core/series.py
Outdated
|
|
||
| The n largest elements where n=3. Default keep value is 'first' so | ||
| Malta will | ||
| be kept. |
There was a problem hiding this comment.
Can be on the same line as "Malta will"
pandas/core/series.py
Outdated
| Malta 434000 | ||
| dtype: int64 | ||
|
|
||
| The n largest elements where n=3 and keeping the last duplicates. |
pandas/core/series.py
Outdated
| @@ -2789,16 +2837,19 @@ def nsmallest(self, n=5, keep='first'): | |||
| Parameters | |||
| ---------- | |||
| n : int | |||
There was a problem hiding this comment.
can you add the default here?
pandas/core/series.py
Outdated
| - ``last`` : take the last occurrence. | ||
| n : int, default 5 | ||
| Return this many descending sorted values. | ||
| keep : str, default 'first' |
There was a problem hiding this comment.
I think it's better to keep {'first', 'last', 'all'}, as I don't think there is any other value allowed. That applies to both docstrings.
| @@ -2762,23 +2765,68 @@ def nlargest(self, n=5, keep='first'): | |||
|
|
|||
| See Also | |||
| -------- | |||
| Series.nsmallest | |||
| Series.nsmallest: Get the `n` smallest elements. | |||
There was a problem hiding this comment.
I'd add sort_values and head here, as they're mentioned in the notes. In both docstrings.
|
Passed on travis. Merging. |
git diff upstream/master -u -- "*.py" | flake8 --diffBased on #22459. Fix the docstring for Series.nsmallest and Series.nlargest. I did both together since the same example could be used. I removed both from the skipped doctests in
ci/doctests.sh.I also found out about the "all" option for the
keepparameter which was surprisingly not documented.