Bug : Fixes #20911 #24467

cgangwar11 · 2018-12-28T13:54:16Z

closes DataFrame.clip() bug when bound a frame when columns not sorted #20911
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Before

In [1]: import pandas as pd                                                                   

In [2]: df1 = pd.DataFrame([[1., 0.], [3., 0.]], columns=['A', 'B']) 
   ...:                                                                                       

In [3]: df2 = pd.DataFrame([[100., 1.], [100., 2.]], columns=['B', 'A']) 
   ...:                                                                                       

In [4]: df1                                                                                   
Out[4]: 
     A    B
0  1.0  0.0
1  3.0  0.0

In [5]: df2                                                                                   
Out[5]: 
       B    A
0  100.0  1.0
1  100.0  2.0

In [6]: df1.clip(lower=0,upper=df2)                                                           
Out[6]: 
       A    B
0    1.0  0.0
1  100.0  0.0

After

In [5]: df1.clip(lower=0,upper=df2)                                                           
Out[5]: 
     A    B
0  1.0  0.0
1  2.0  0.0

Passing threshold as it is instead of numpy array to preserve the columns order.

Additional test cases to validate the fix

pep8speaks · 2018-12-28T13:54:22Z

Hello @cgangwar11! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on December 28, 2018 at 22:56 Hours UTC

jreback

pls add a whatsnew note

jreback · 2018-12-28T13:56:04Z

pandas/tests/frame/test_analytics.py

@@ -1964,6 +1964,13 @@ def test_clip_against_frame(self, axis):
        tm.assert_frame_equal(clipped_df[lb_mask], lb[lb_mask])
        tm.assert_frame_equal(clipped_df[ub_mask], ub[ub_mask])
        tm.assert_frame_equal(clipped_df[mask], df[mask])
+         # GH 20911, clipping now preserves types       


make this a new test

jreback · 2018-12-28T13:56:25Z

pandas/tests/frame/test_analytics.py

@@ -1964,6 +1964,13 @@ def test_clip_against_frame(self, axis):
        tm.assert_frame_equal(clipped_df[lb_mask], lb[lb_mask])
        tm.assert_frame_equal(clipped_df[ub_mask], ub[ub_mask])
        tm.assert_frame_equal(clipped_df[mask], df[mask])
+         # GH 20911, clipping now preserves types       
+        df1 = DataFrame(np.random.randn(1000,4), columns=['A', 'B','C','D'])
+        df2 = DataFrame(np.random.randn(1000,4), columns=['D', 'A','B','C'])


you have some lint errors.

jreback · 2018-12-28T13:56:37Z

pandas/tests/frame/test_analytics.py

+        df1 = DataFrame(np.random.randn(1000,4), columns=['A', 'B','C','D'])
+        df2 = DataFrame(np.random.randn(1000,4), columns=['D', 'A','B','C'])
+
+        res1 = df1.clip(lower=0,upper=df2)


use result= and expected=

will do pardon me for such a rookie mistake....

codecov · 2018-12-28T14:38:26Z

Codecov Report

Merging #24467 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #24467   +/-   ##
=======================================
  Coverage    92.3%    92.3%           
=======================================
  Files         163      163           
  Lines       51969    51969           
=======================================
  Hits        47968    47968           
  Misses       4001     4001

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

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.62% <100%> (ø)`	⬆️

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 aa1549f...3dabccf. Read the comment docs.

codecov · 2018-12-28T14:38:26Z

Codecov Report

Merging #24467 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #24467      +/-   ##
==========================================
- Coverage   92.31%   92.31%   -0.01%     
==========================================
  Files         165      165              
  Lines       52252    52204      -48     
==========================================
- Hits        48237    48192      -45     
+ Misses       4015     4012       -3

Flag	Coverage Δ
#multiple	`90.73% <100%> (-0.01%)`	⬇️
#single	`42.96% <0%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.62% <100%> (ø)`	⬆️
pandas/core/arrays/timedeltas.py	`87.36% <0%> (-0.27%)`	⬇️
pandas/util/testing.py	`87.75% <0%> (-0.1%)`	⬇️
pandas/core/arrays/period.py	`98.42% <0%> (-0.05%)`	⬇️
pandas/core/indexes/datetimelike.py	`97.53% <0%> (ø)`	⬆️
pandas/core/arrays/base.py	`98.23% <0%> (+0.03%)`	⬆️
pandas/core/arrays/sparse.py	`92.17% <0%> (+0.06%)`	⬆️
pandas/core/arrays/datetimes.py	`98.39% <0%> (+0.13%)`	⬆️
pandas/core/arrays/datetimelike.py	`96.08% <0%> (+0.43%)`	⬆️

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 c1af4f5...9ee4b89. Read the comment docs.

jreback

pls add a whatsnew entry

jreback · 2018-12-28T15:59:34Z

pandas/tests/frame/test_analytics.py

+        # GH 20911
+        df1 = DataFrame(np.random.randn(1000, 4), columns=['A', 'B', 'C', 'D'])
+        df2 = DataFrame(np.random.randn(1000, 4), columns=['D', 'A', 'B', 'C'])
+        result = df1.clip(lower=0, upper=df2)


can you try this with lower as well

…r cliping

jreback · 2018-12-28T23:02:15Z

lgtm ping on green

cgangwar11 · 2018-12-28T23:19:52Z

Better Example

Before`

In [11]: import pandas as pd
    ...: 
    ...: df1 = pd.DataFrame([[50., 60.,70.], [80., 90.,100.]], columns=['A', 'B','C'])
    ...: 
    ...: upper = pd.DataFrame([[50., 80.,49.], [100.,100.,90.]], columns=['B', 'C','A'])
    ...: 
    ...: lower = pd.DataFrame([[60., 40.,35.], [90.,95.,85.]], columns=['C', 'B','A'])
    ...: 
    ...: 

In [12]: df1
Out[12]: 
      A     B      C
0  50.0  60.0   70.0
1  80.0  90.0  100.0

In [16]: lower[df1.columns]
Out[16]: 
      A     B     C
0  35.0  40.0  60.0
1  85.0  95.0  90.0

In [17]: upper[df1.columns]
Out[17]: 
      A      B      C
0  49.0   50.0   80.0
1  90.0  100.0  100.0

In [18]: df1.clip(lower=lower,upper=upper)
Out[18]: 
      A     B      C
0  50.0  80.0   70.0
1  90.0  95.0  100.0

After

In [8]: (df1.clip(lower=lower, upper=upper))
Out[8]:
      A     B      C
0  49.0  50.0   70.0
1  85.0  95.0  100.0

cgangwar11 · 2018-12-28T23:32:48Z

@jreback it's green now

jreback · 2018-12-28T23:36:14Z

thanks @cgangwar11

cgangwar11 added 2 commits December 28, 2018 19:01

BUG GH20911

9605547

Passing threshold as it is instead of numpy array to preserve the columns order.

TST GH20911

3dabccf

Additional test cases to validate the fix

jreback requested changes Dec 28, 2018

View reviewed changes

jreback added Bug Numeric Operations Arithmetic, Comparison, and Logical operations labels Dec 28, 2018

cgangwar11 changed the title ~~Local/20911~~ Bug : Fixes #20911 Dec 28, 2018

cgangwar11 added 2 commits December 28, 2018 20:16

Seperated test case and solved the linting errors.

cf86de4

Merge branch 'master' into local/20911

0f5faf8

jreback requested changes Dec 28, 2018

View reviewed changes

cgangwar11 added 3 commits December 29, 2018 02:42

Merge branch 'master' into local/20911

fd20ff8

Added test cases for lower cliping as well as combined lower and uppe…

2ddb60f

…r cliping

whatsnew note added

9ee4b89

jreback added this to the 0.24.0 milestone Dec 28, 2018

jreback approved these changes Dec 28, 2018

View reviewed changes

jreback merged commit 4f1c1dc into pandas-dev:master Dec 28, 2018

cgangwar11 deleted the local/20911 branch January 3, 2019 18:16

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

Bug : Fixes pandas-dev#20911 (pandas-dev#24467)

f45dce2

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

Bug : Fixes pandas-dev#20911 (pandas-dev#24467)

003e7e7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug : Fixes #20911 #24467

Bug : Fixes #20911 #24467

cgangwar11 commented Dec 28, 2018 •

edited

Loading

pep8speaks commented Dec 28, 2018 •

edited

Loading

jreback left a comment

jreback Dec 28, 2018

jreback Dec 28, 2018

jreback Dec 28, 2018

cgangwar11 Dec 28, 2018

codecov bot commented Dec 28, 2018

codecov bot commented Dec 28, 2018 •

edited

Loading

jreback left a comment

jreback Dec 28, 2018

jreback commented Dec 28, 2018

cgangwar11 commented Dec 28, 2018

cgangwar11 commented Dec 28, 2018

jreback commented Dec 28, 2018

Bug : Fixes #20911 #24467

Bug : Fixes #20911 #24467

Conversation

cgangwar11 commented Dec 28, 2018 • edited Loading

pep8speaks commented Dec 28, 2018 • edited Loading

Comment last updated on December 28, 2018 at 22:56 Hours UTC

jreback left a comment

Choose a reason for hiding this comment

jreback Dec 28, 2018

Choose a reason for hiding this comment

jreback Dec 28, 2018

Choose a reason for hiding this comment

jreback Dec 28, 2018

Choose a reason for hiding this comment

cgangwar11 Dec 28, 2018

Choose a reason for hiding this comment

codecov bot commented Dec 28, 2018

Codecov Report

codecov bot commented Dec 28, 2018 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

jreback Dec 28, 2018

Choose a reason for hiding this comment

jreback commented Dec 28, 2018

cgangwar11 commented Dec 28, 2018

Better Example

cgangwar11 commented Dec 28, 2018

jreback commented Dec 28, 2018

cgangwar11 commented Dec 28, 2018 •

edited

Loading

pep8speaks commented Dec 28, 2018 •

edited

Loading

codecov bot commented Dec 28, 2018 •

edited

Loading