DOC: Fix errors in pandas.Series.argmax #32019
Conversation
pandas/core/base.py
Outdated
| @@ -942,7 +948,22 @@ def argmax(self, axis=None, skipna=True, *args, **kwargs): | |||
|
|
|||
| See Also | |||
| -------- | |||
| numpy.ndarray.argmax | |||
| numpy.ndarray.argmax : Returns the indices of the maximum values along an axis. | |||
There was a problem hiding this comment.
Do we have a pandas argmin that is worth adding here?
There was a problem hiding this comment.
See Also
--------
numpy.ndarray.argmax : Returns the indices of the maximum values along an axis.
pandas.Series.argmin : Return a ndarray of the minimum argument indexer.
Would this be sufficient?
There was a problem hiding this comment.
Looks good. I'm not sure if the pandas. prefix is needed, may be you can check other docstrings and see.
And for the descriptions, it could make sense to say something like "Equivalent method for numpy arrays.", "Same, but returning the minimum". I think for users could be easier to understand what those methods do.
There was a problem hiding this comment.
Yes, I've noticed through scripts/validate_docstrings.py that I could safely omit pandas. to match the guidelines.
I agree with your proposed descriptions, as current description is redundant with each function description
pandas/core/base.py
Outdated
| Examples | ||
| -------- | ||
| >>> s = pd.Series(data=[1, None, 5, 4, 5], | ||
| ... index=['A', 'B', 'C', 'D', 'E']) |
There was a problem hiding this comment.
Do you think you can find some data that looks more "real". In Series.combine we've got a small Series with animal speeds that I think could be more appropriate to illustrate this: pd.Series({'falcon': 345.0, 'eagle': 200.0, 'duck': 30.0})
Also, may be worth adding a note saying that the 2 means that largest value is in the third element, since it's zero-indexed.
There was a problem hiding this comment.
Sure, I'm gonna use cereal calories dataset as an example and adding that note.
| @@ -942,7 +948,27 @@ def argmax(self, axis=None, skipna=True, *args, **kwargs): | |||
|
|
|||
| See Also | |||
| -------- | |||
| numpy.ndarray.argmax | |||
| numpy.ndarray.argmax : Equivalent method for numpy arrays. | |||
| Series.argmin : Similar method, but returning the minimum. | |||
There was a problem hiding this comment.
Series.idxmax, Series.idxmin might also make sense here
pandas/core/base.py
Outdated
| Consider dataset containing cereal calories | ||
|
|
||
| >>> s = pd.Series({'Corn Flakes': 100.0, 'Almond Delight': 110.0, | ||
| ... 'Cinnamon Toast Crunch': 120.0, 'Cocoa Puff': 110.0}) |
There was a problem hiding this comment.
@datapythonista do we care about getting an extra space before the quote here?
There was a problem hiding this comment.
Is this what you're looking for?
>>> s = pd.Series({'Corn Flakes' : 100.0, 'Almond Delight' : 110.0,
... 'Cinnamon Toast Crunch' : 120.0, 'Cocoa Puff' : 110.0})
There was a problem hiding this comment.
i meant an extra space after the ... on the second line, so that the single-quote in Cinnamon Toast Crunch aligned with the single-quote in Corn Flakes. Feel free to ignore, unless @datapythonista chimes in on the policy
pandas/core/base.py
Outdated
| @@ -929,11 +929,17 @@ def argmax(self, axis=None, skipna=True, *args, **kwargs): | |||
| """ | |||
| Return an ndarray of the maximum argument indexer. | |||
|
|
|||
| If multiple values equal the maximum, the first row label with that | |||
There was a problem hiding this comment.
If the maximum is achieved in multiple locations, the first such location is returned.
There was a problem hiding this comment.
Do you think we should go with location instead of row position?
There was a problem hiding this comment.
"row position" sounds good. Definitely not "row label".
pandas/core/base.py
Outdated
| @@ -929,11 +929,17 @@ def argmax(self, axis=None, skipna=True, *args, **kwargs): | |||
| """ | |||
| Return an ndarray of the maximum argument indexer. | |||
There was a problem hiding this comment.
is this sentence accurate? it looks like we return an int, not ndarray
There was a problem hiding this comment.
(also relevant for the Returns section)
There was a problem hiding this comment.
Do you think this okay?
Return row position of the maximum argument indexer.
And also change for the Returns section:
Returns
-------
int
row position of the maximum values.
There was a problem hiding this comment.
"maximum argument indexer" is awkward. how about "Returns int position of the largest value in the Series"
There was a problem hiding this comment.
"maximum argument indexer" is awkward. how about "Returns int position of the largest value in the Series"
I think this one is easier to understand.
|
Thanks for the nice docstring @farhanreynaldo |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffoutput of
python scripts/validate_docstrings.py pandas.Series.argmax: