CLN: enforce the deprecation of the Series.argsort NA behavior
#58232
Conversation
pandas/core/series.py
Outdated
| "last instead of set to -1.", | ||
| FutureWarning, | ||
| stacklevel=find_stack_level(), | ||
| raise TypeError( |
There was a problem hiding this comment.
Based on the TODO comment, it appears this should dispatch to self.array.argsort
Also could you see if it closes those associated issues referenced in the comment?
There was a problem hiding this comment.
Based on the
TODOcomment, it appears this should dispatch toself.array.argsort
sorry, it’s my mistake, I dispatched Series.argsort to EA.argsort.
Also could you see if it closes those associated issues referenced in the comment?
Yes, it will close issues referenced in the comment.
@mroeschke could you please take a look at this PR? I think CI failures are unrelated to my changes.
There was a problem hiding this comment.
cc @jbrockmendel for a double check
| res = np.argsort(ser).values | ||
| expected = np.argsort(np.array(ser)) | ||
| tm.assert_numpy_array_equal(res, expected) | ||
|
|
||
| # with missing values |
There was a problem hiding this comment.
is there a reason for deleting this part of the test instead of updating it?
| result = pd.Series(data_missing_for_sorting).argsort() | ||
| expected = pd.Series(np.array([1, -1, 0], dtype=np.intp)) | ||
| result = pd.Series(data_missing_for_sorting).argsort() | ||
| expected = pd.Series(np.array([2, 0, 1], dtype=np.intp)) |
There was a problem hiding this comment.
are NAs supposed to be sorted to the front or the end? IIUC this is putting them at the front?
There was a problem hiding this comment.
I think this is correct as the NA should be put at the end according to the deprecation message.
The fixture says
This should be three items [B, NA, A] with
A < B and NA missing.
There was a problem hiding this comment.
Ok, I think I was confusing this with ‘rank’
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Thanks @natmokval |
…das-dev#58232) * enforce deprecation of the Series.argsort NA behavior * remove comments * add a note to v3.0.0 * correct def argsort and tests * correct def argsort/tests * fix pre-commit error * Restore numpy test --------- Co-authored-by: Matthew Roeschke <[email protected]>
|
thanks @mroeschke for reviewing this PR |
xref #54219
enforced the deprecation of the
Series.argsortNAbehavior.closes #12694
closes #43840