ENH: Allow keep='all' for nlargest/nsmallest #21650

gfyoung · 2018-06-27T07:14:39Z

Title is self-explanatory.

codecov · 2018-06-27T09:46:28Z

Codecov Report

Merging #21650 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #21650      +/-   ##
==========================================
+ Coverage    91.9%    91.9%   +<.01%     
==========================================
  Files         154      154              
  Lines       49555    49557       +2     
==========================================
+ Hits        45542    45544       +2     
  Misses       4013     4013

Flag	Coverage Δ
#multiple	`90.27% <100%> (ø)`	⬆️
#single	`42.03% <0%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/frame.py	`97.19% <ø> (ø)`	⬆️
pandas/core/algorithms.py	`94.85% <100%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0f978d...4998050. Read the comment docs.

jschendel · 2018-06-27T13:30:38Z

pandas/tests/series/test_analytics.py

+        s = Series([10, 9, 8, 7, 7, 7, 7, 6])
+        result = s.nlargest(4, keep='all')
+        expected = Series([10, 9, 8, 7, 7, 7, 7])
+        print(result, expected)


print statement should be removed

jschendel · 2018-06-27T13:36:51Z

doc/source/whatsnew/v0.24.0.txt

@@ -24,6 +24,7 @@ Other Enhancements
  <https://pandas-gbq.readthedocs.io/en/latest/changelog.html#changelog-0-5-0>`__.
  (:issue:`21627`)
 - New method :meth:`HDFStore.walk` will recursively walk the group hierarchy of an HDF5 file (:issue:`10932`)
+- :func:`Series` / :func:`DataFrame` methods :func:`nlargest` / :func:`nsmallest` now accept the value 'all' for the `keep` argument. This keeps all ties for the nth largest/smallest value (:issue:`16818`)


I don't think these will link properly if the class is separated from the method (I could be wrong though), and I think you'll need to explicitly write out the variations, e.g. :meth:`Series.nlargest`.

gfyoung · 2018-06-27T16:37:49Z

@jschendel : Thanks for comments! TBH, I didn't really check the PR diff before trying to apply it on master. 😄 I'll get to these shortly.

gfyoung · 2018-06-27T22:59:53Z

@jreback @jschendel : Comments addressed, and all is passing. PTAL.

jschendel · 2018-06-27T23:47:04Z

Can you add 'all' to the list of params for the Series and DataFrame asv's related to NSort:

pandas/asv_bench/benchmarks/series_methods.py

Lines 41 to 44 in a620e72

    
           class NSort(object): 
        
               goal_time = 0.2 
        
               params = ['last', 'first']

pandas/asv_bench/benchmarks/frame_methods.py

Lines 501 to 504 in a620e72

    
           class NSort(object): 
        
               goal_time = 0.2 
        
               params = ['first', 'last']

I don't think this change will have any perf impact, so no need to run the asv's, but might be nice to have benchmarks in place for this.

LGTM otherwise.

gfyoung · 2018-06-28T00:15:18Z

@jschendel : Yep, done.

jreback

lgtm. very minor comments. merge on green.

jreback · 2018-06-28T10:32:14Z

pandas/core/frame.py

+        4  8  e  4.0
+
+        When using ``keep='all'``, all duplicate items are maintained
+        >>> df.nsmallest(3, 'a', keep='all')


nitpick, blank line here

jreback · 2018-06-28T10:32:47Z

pandas/core/frame.py

+        ``TypeError``:
+
+        >>> df.nsmallest(3, 'b')
+        Traceback (most recent call last):


jreback · 2018-06-28T10:33:37Z

pandas/tests/frame/test_analytics.py

@@ -2461,6 +2461,22 @@ def test_n_duplicate_index(self, df_duplicates, n, order):
        expected = df.sort_values(order, ascending=False).head(n)
        tm.assert_frame_equal(result, expected)

+    def test_keep_all_ties(self):


can you name: test_duplicate_keep_all_ties

side issue: I think make sense to split out duplicate tests to separate file (can you make an issue / PR if you feel!)

jreback · 2018-06-28T10:33:48Z

pandas/tests/series/test_analytics.py

@@ -2082,6 +2082,17 @@ def test_boundary_datetimelike(self, nselect_method, dtype):
        vals = [min_val + 1, min_val + 2, max_val - 1, max_val, min_val]
        assert_check_nselect_boundary(vals, dtype, nselect_method)

+    def test_keep_all_ties(self):


same naming here

jreback · 2018-06-28T10:34:07Z

doc/source/whatsnew/v0.24.0.txt

@@ -24,6 +24,7 @@ Other Enhancements
  <https://pandas-gbq.readthedocs.io/en/latest/changelog.html#changelog-0-5-0>`__.
  (:issue:`21627`)
 - New method :meth:`HDFStore.walk` will recursively walk the group hierarchy of an HDF5 file (:issue:`10932`)
+- :meth:`Series.nlargest`, :meth:`Series.nsmallest`, :meth:`DataFrame.nlargest`, and :meth:`DataFrame.nsmallest` now accept the value ``"all"`` for the ``keep` argument. This keeps all ties for the nth largest/smallest value (:issue:`16818`)


can you add both issue numbers here?

The second issue number #18656 is an actually the old PR. I mentioned it for completeness.

Closes pandas-devgh-16818. Closes pandas-devgh-18656.

gfyoung · 2018-06-28T21:20:18Z

All is green, so merging.

Closes pandas-devgh-16818. Closes pandas-devgh-18656.

gfyoung added Enhancement API Design Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff labels Jun 27, 2018

gfyoung added this to the 0.24.0 milestone Jun 27, 2018

gfyoung mentioned this pull request Jun 27, 2018

added option keep=False to nlargests/nsmallest #18656

Closed

4 tasks

gfyoung requested a review from jreback June 27, 2018 07:16

jschendel reviewed Jun 27, 2018

View reviewed changes

gfyoung force-pushed the nlargest-keep-last branch from 8d3b883 to 4b7f199 Compare June 27, 2018 16:47

gfyoung force-pushed the nlargest-keep-last branch from 4b7f199 to a07c686 Compare June 28, 2018 00:15

jreback approved these changes Jun 28, 2018

View reviewed changes

ENH: Allow keep='all' for nlargest/nsmallest

4998050

Closes pandas-devgh-16818. Closes pandas-devgh-18656.

gfyoung force-pushed the nlargest-keep-last branch from a07c686 to 4998050 Compare June 28, 2018 17:01

gfyoung merged commit 0801b8c into pandas-dev:master Jun 28, 2018

gfyoung deleted the nlargest-keep-last branch June 28, 2018 21:20

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

ENH: Allow keep='all' for nlargest/nsmallest (pandas-dev#21650)

db18fd6

Closes pandas-devgh-16818. Closes pandas-devgh-18656.

mattf mentioned this pull request Jan 10, 2023

[FEA] Allow keep='all' for nlargest/nsmallest rapidsai/cudf#12513

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Allow keep='all' for nlargest/nsmallest #21650

ENH: Allow keep='all' for nlargest/nsmallest #21650

gfyoung commented Jun 27, 2018

codecov bot commented Jun 27, 2018 •

edited

Loading

jschendel Jun 27, 2018

gfyoung Jun 27, 2018

jschendel Jun 27, 2018

gfyoung Jun 27, 2018

gfyoung commented Jun 27, 2018 •

edited

Loading

gfyoung commented Jun 27, 2018

jschendel commented Jun 27, 2018 •

edited

Loading

gfyoung commented Jun 28, 2018

jreback left a comment

jreback Jun 28, 2018

gfyoung Jun 28, 2018

jreback Jun 28, 2018

gfyoung Jun 28, 2018

jreback Jun 28, 2018

gfyoung Jun 28, 2018

jreback Jun 28, 2018

gfyoung Jun 28, 2018

jreback Jun 28, 2018

gfyoung Jun 28, 2018

gfyoung commented Jun 28, 2018

ENH: Allow keep='all' for nlargest/nsmallest #21650

ENH: Allow keep='all' for nlargest/nsmallest #21650

Conversation

gfyoung commented Jun 27, 2018

codecov bot commented Jun 27, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung commented Jun 27, 2018 • edited Loading

gfyoung commented Jun 27, 2018

jschendel commented Jun 27, 2018 • edited Loading

gfyoung commented Jun 28, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung commented Jun 28, 2018

codecov bot commented Jun 27, 2018 •

edited

Loading

gfyoung commented Jun 27, 2018 •

edited

Loading

jschendel commented Jun 27, 2018 •

edited

Loading