BUG: nlargest/nsmallest can now consider nan values like sort_values(ascending=True).head(n) #43060
Conversation
|
Hello @usersblock! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-09-07 07:41:45 UTC |
|
@usersblock i've closed #42997 as a duplicate of #28984, can you update the references to the issue also include the code sample from #28984 as a test. |
simonjayhawkins
added
the
Numeric Operations
|
Updated reference and added new test to test_nlargest.py in frames\methods |
There was a problem hiding this comment.
@usersblock can you add a release note to doc/source/whatsnew/v1.4.0.rst
| tm.assert_series_equal(ser.nlargest(), ser.iloc[[4, 0, 3, 2]]) | ||
| tm.assert_series_equal(ser.nsmallest(), ser.iloc[[2, 3, 0, 4]]) | ||
| tm.assert_series_equal(ser.nlargest(), ser.iloc[[4, 0, 3, 2, 1]]) | ||
| tm.assert_series_equal(ser.nsmallest(), ser.iloc[[2, 3, 0, 4, 1]]) |
There was a problem hiding this comment.
can you rewrite this test using result= and expected= while making changes in this test.
There was a problem hiding this comment.
Added result/expected and added entry into the doc
|
ok can you add a replica of the test in the OP for |
pandas/core/algorithms.py
Outdated
|
|
||
| if self.keep == "last": | ||
| # reverse indices | ||
| inds = narr - 1 - inds | ||
|
|
||
| return dropped.iloc[inds] | ||
| return concat([dropped.iloc[inds], nan_index])[:findex] |
There was a problem hiding this comment.
dont' you need to use .iloc here? as this will not be a positional lookup (it might work with a range index though)
There was a problem hiding this comment.
Should be done now
|
thanks @usersblock very nice! |
nsmallest/nlargest are now consistent with x.sort_values(ascending=False).head(n)/x.sort_values(ascending=True).head(n)
Edit: I've changed test_nlargest_misc(self) in \tests\serie\methods\test_nlargest.py to reflect the fact that Nans are included