ERR: qcut uniquess checking (try 2) #15000

ashishsingal1 · 2016-12-27T22:56:15Z

closes qcut() should make sure the bins bounderies are unique before passing them to _bins_to_cuts #7751
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

Add option to drop non-unique bins.

ashishsingal1 · 2016-12-27T22:57:15Z

My second try on #14455 -- I wasn't able to rebase this one.

codecov-io · 2016-12-28T13:30:46Z

Current coverage is 84.78% (diff: 87.50%)

Merging #15000 into master will decrease coverage by <.01%

@@             master     #15000   diff @@
==========================================
  Files           145        145          
  Lines         51090      51095     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43315      43319     +4   
- Misses         7775       7776     +1   
  Partials          0          0

Powered by Codecov. Last update 7f0eefc...698b4ec

jreback · 2016-12-28T13:42:16Z

pandas/tools/tile.py

@@ -151,6 +151,8 @@ def qcut(x, q, labels=None, retbins=False, precision=3):
        as a scalar.
    precision : int
        The precision at which to store and display the bins labels
+    duplicates : {'raise', 'drop'}, optional
+        If binned edges are not unique, raise ValueError or drop non-uniques.


add a versionadded tag (0.20.0)

jreback · 2016-12-28T13:42:53Z

pandas/tools/tile.py

+        raise ValueError("invalid value for 'duplicates' parameter, "
+                         + "valid options are: raise, drop")
+
+    if len(algos.unique(bins)) < len(bins):


do

bins = algos.unique(bins)

first (so you have it already for below)

Not sure I understand -- if I do that before line 213, how would I compare len of bins before and after in line 207 and how would I output the original non unique edges in 209 in case duplicates='raise'?

call it something else so it doesn't overwrite this

Done. Is this for performance reasons?

jreback · 2016-12-28T13:43:18Z

pls add a whatsnew in 0.20.0 (other enhancements)

jorisvandenbossche · 2016-12-28T14:37:13Z

@ashishsingal1 For future reference, it is not needed to start a new PR, you can always clean up locally and just force push to the same branch in your fork, and this PR will be updated automatically. In fact, we rather much prefer that instead of opening a new PR, to keep discussions gathered at one place. If you need help with those git aspects, don't hesitate to ask!

Add unique_bins for performance

jorisvandenbossche

Looking good already! Added some more comments.

jorisvandenbossche · 2016-12-29T00:15:59Z

pandas/tools/tile.py

@@ -151,6 +151,9 @@ def qcut(x, q, labels=None, retbins=False, precision=3):
        as a scalar.
    precision : int
        The precision at which to store and display the bins labels
+    duplicates : {'raise', 'drop'}, optional
+        If binned edges are not unique, raise ValueError or drop non-uniques.
+        .. versionadded:: 0.20.0


There needs to be a whiteline above this one (rst specifics ...)

jorisvandenbossche · 2016-12-29T00:16:17Z

pandas/tools/tile.py

@@ -151,6 +151,9 @@ def qcut(x, q, labels=None, retbins=False, precision=3):
        as a scalar.
    precision : int
        The precision at which to store and display the bins labels
+    duplicates : {'raise', 'drop'}, optional
+        If binned edges are not unique, raise ValueError or drop non-uniques.


"binned edges" -> bin edges ?

jorisvandenbossche · 2016-12-29T00:17:30Z

pandas/tools/tests/test_tile.py

+        values = [0, 0, 0, 0, 1, 2, 3]
+        cats = qcut(values, 3, duplicates='drop')
+        ex_levels = ['[0, 1]', '(1, 3]']
+        self.assertTrue((cats.categories == ex_levels).all())


Can you add a test for duplicates='raise' (the default) as well? (assert that it raises a ValueError)

jorisvandenbossche · 2016-12-29T00:21:03Z

pandas/tools/tile.py

+
+    if duplicates not in ['raise', 'drop']:
+        raise ValueError("invalid value for 'duplicates' parameter, "
+                         + "valid options are: raise, drop")


the + is not needed here. The strings will automatically be concatenated due to Python's implied line continuation inside parentheses

jorisvandenbossche · 2016-12-29T00:22:53Z

pandas/tools/tile.py

+        if duplicates == 'raise':
+            raise ValueError('Bin edges must be unique: %s' % repr(bins) +
+                             ' You can drop duplicate edges ' +
+                             'by setting \'duplicates\' param')


Same here about the +'s. Further, if you use " for the string, you don not need to escape the ' inside the string.
Lastly, can you put the injection of "repr(bins)" at the end of the string (and maybe also use .format instead of %)

…ise'

Line too long code formatting fix.

jorisvandenbossche

@ashishsingal1 Thanks for the update. Looking good! Just added some small comments on the docs

jorisvandenbossche · 2016-12-29T12:08:14Z

pandas/tools/tile.py

@@ -151,6 +151,9 @@ def qcut(x, q, labels=None, retbins=False, precision=3):
        as a scalar.
    precision : int
        The precision at which to store and display the bins labels
+    duplicates : {'raise', 'drop'}, optional
+        If bin edges are not unique, raise ValueError or drop non-uniques.
+        .. versionadded:: 0.20.0


There needs to be a blank line above .. versionadded ... (rst specifics ..)

jorisvandenbossche · 2016-12-29T12:10:17Z

pandas/tools/tile.py

@@ -151,6 +151,9 @@ def qcut(x, q, labels=None, retbins=False, precision=3):
        as a scalar.
    precision : int
        The precision at which to store and display the bins labels
+    duplicates : {'raise', 'drop'}, optional


can you add "default 'raise'"

or you can also add it in the description like "Default is to raise an error" (one of both is good)

Docstring update

jreback · 2016-12-29T13:57:19Z

doc/source/whatsnew/v0.20.0.txt

@@ -105,6 +105,7 @@ Other enhancements
  of sorting or an incorrect key. See :ref:`here <advanced.unsorted>`

 - ``pd.cut`` and ``pd.qcut`` now support datetime64 and timedelta64 dtypes (:issue:`14714`, :issue:`14798`)
+- ``pd.qcut`` can optionally remove duplicate edges instead of throwing an error (:issue:`7751`)


pd.cut/qcut have gained the duplicates kw to control whether to raise on duplicated edges.

Is this relevant for cut or just qcut?

Deleting whitespace.

ashishsingal1 · 2016-12-30T13:13:16Z

@jreback @jorisvandenbossche anything left for this PR on my side?

jreback · 2016-12-30T19:02:08Z

thanks!

adding 'duplicates' option to qcut

bee981c

ashishsingal1 added 2 commits December 27, 2016 20:34

fixing duplicates check

a2dd8ce

Update tile.py

0b8efeb

jreback reviewed Dec 28, 2016

View reviewed changes

jreback added Error Reporting Incorrect or improved errors from pandas Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Dec 28, 2016

Update tile.py

3f98abc

jorisvandenbossche added this to the 0.20.0 milestone Dec 28, 2016

ashishsingal1 added 3 commits December 28, 2016 10:12

Update test_tile.py

1ce77d0

Update tile.py

2161518

Add unique_bins for performance

Update v0.20.0.txt

3dbc416

jorisvandenbossche reviewed Dec 29, 2016

View reviewed changes

ashishsingal1 added 2 commits December 28, 2016 21:25

added duplicates='raise' test. other fixes to qcut for duplicates='ra…

2c5bc35

…ise'

Update tile.py

221c0b3

Line too long code formatting fix.

jorisvandenbossche reviewed Dec 29, 2016

View reviewed changes

Update tile.py

42bf482

Docstring update

jreback reviewed Dec 29, 2016

View reviewed changes

Update v0.20.0.txt

b6bf401

jorisvandenbossche approved these changes Dec 29, 2016

View reviewed changes

Update tile.py

698b4ec

Deleting whitespace.

jreback closed this in 8051d61 Dec 30, 2016

jorisvandenbossche mentioned this pull request Jan 5, 2017

qcut can fail for highly discontinuous data distributions #15069

Open

luca-s mentioned this pull request Jan 30, 2017

Bin edges must be unique quantopian/alphalens#131

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ERR: qcut uniquess checking (try 2) #15000

ERR: qcut uniquess checking (try 2) #15000

ashishsingal1 commented Dec 27, 2016 •

edited

ashishsingal1 commented Dec 27, 2016

codecov-io commented Dec 28, 2016 •

edited

jreback Dec 28, 2016

jreback Dec 28, 2016

ashishsingal1 Dec 28, 2016

jreback Dec 28, 2016

ashishsingal1 Dec 29, 2016

jreback commented Dec 28, 2016

jorisvandenbossche commented Dec 28, 2016

jorisvandenbossche left a comment

jorisvandenbossche Dec 29, 2016

jorisvandenbossche Dec 29, 2016

jorisvandenbossche Dec 29, 2016

jorisvandenbossche Dec 29, 2016

jorisvandenbossche Dec 29, 2016

jorisvandenbossche left a comment

jorisvandenbossche Dec 29, 2016

jorisvandenbossche Dec 29, 2016

jorisvandenbossche Dec 29, 2016

jreback Dec 29, 2016

ashishsingal1 Dec 29, 2016

ashishsingal1 commented Dec 30, 2016

jreback commented Dec 30, 2016

ERR: qcut uniquess checking (try 2) #15000

ERR: qcut uniquess checking (try 2) #15000

Conversation

ashishsingal1 commented Dec 27, 2016 • edited

ashishsingal1 commented Dec 27, 2016

codecov-io commented Dec 28, 2016 • edited

Current coverage is 84.78% (diff: 87.50%)

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

jreback commented Dec 28, 2016

jorisvandenbossche commented Dec 28, 2016

jorisvandenbossche 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

jorisvandenbossche 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

ashishsingal1 commented Dec 30, 2016

jreback commented Dec 30, 2016

ashishsingal1 commented Dec 27, 2016 •

edited

codecov-io commented Dec 28, 2016 •

edited