Skip to content

Commit

Permalink
BUG: clip should handle null values
Browse files Browse the repository at this point in the history
closes #17276

Author: Michael Gasvoda <mgasvoda@mercatus.gmu.edu>
Author: mgasvoda <mgasvoda01@gmail.com>

Closes #17288 from mgasvoda/master and squashes the following commits:

a1dbdf2 [mgasvoda] Merge branch 'master' into master
9333952 [Michael Gasvoda] Checking output of tests
4e0464e [Michael Gasvoda] fixing whatsnew text
c442040 [Michael Gasvoda] formatting fixes
7e23678 [Michael Gasvoda] formatting updates
781ea72 [Michael Gasvoda] whatsnew entry
d9627fe [Michael Gasvoda] adding clip tests
9aa0159 [Michael Gasvoda] Treating na values as none for clips
  • Loading branch information
mgasvoda authored and jreback committed Aug 21, 2017
1 parent eff1f88 commit 910207f
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 21 deletions.
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.21.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ Other Enhancements




.. _whatsnew_0210.api_breaking:

Backwards incompatible API changes
Expand Down Expand Up @@ -384,6 +383,7 @@ Reshaping
Numeric
^^^^^^^
- Bug in ``.clip()`` with ``axis=1`` and a list-like for ``threshold`` is passed; previously this raised ``ValueError`` (:issue:`15390`)
- :func:`Series.clip()` and :func:`DataFrame.clip()` now treat NA values for upper and lower arguments as ``None`` instead of raising ``ValueError`` (:issue:`17276`).


Categorical
Expand Down
12 changes: 8 additions & 4 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -4741,9 +4741,6 @@ def _clip_with_one_bound(self, threshold, method, axis, inplace):
if axis is not None:
axis = self._get_axis_number(axis)

if np.any(isna(threshold)):
raise ValueError("Cannot use an NA value as a clip threshold")

# method is self.le for upper bound and self.ge for lower bound
if is_scalar(threshold) and is_number(threshold):
if method.__name__ == 'le':
Expand Down Expand Up @@ -4823,6 +4820,14 @@ def clip(self, lower=None, upper=None, axis=None, inplace=False,

axis = nv.validate_clip_with_axis(axis, args, kwargs)

# GH 17276
# numpy doesn't like NaN as a clip value
# so ignore
if np.any(pd.isnull(lower)):
lower = None
if np.any(pd.isnull(upper)):
upper = None

# GH 2747 (arguments were reversed)
if lower is not None and upper is not None:
if is_scalar(lower) and is_scalar(upper):
Expand All @@ -4839,7 +4844,6 @@ def clip(self, lower=None, upper=None, axis=None, inplace=False,
if upper is not None:
if inplace:
result = self

result = result.clip_upper(upper, axis, inplace=inplace)

return result
Expand Down
26 changes: 10 additions & 16 deletions pandas/tests/frame/test_analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -1931,22 +1931,16 @@ def test_clip_against_frame(self, axis):
tm.assert_frame_equal(clipped_df[ub_mask], ub[ub_mask])
tm.assert_frame_equal(clipped_df[mask], df[mask])

def test_clip_na(self):
msg = "Cannot use an NA"
with tm.assert_raises_regex(ValueError, msg):
self.frame.clip(lower=np.nan)

with tm.assert_raises_regex(ValueError, msg):
self.frame.clip(lower=[np.nan])

with tm.assert_raises_regex(ValueError, msg):
self.frame.clip(upper=np.nan)

with tm.assert_raises_regex(ValueError, msg):
self.frame.clip(upper=[np.nan])

with tm.assert_raises_regex(ValueError, msg):
self.frame.clip(lower=np.nan, upper=np.nan)
def test_clip_with_na_args(self):
"""Should process np.nan argument as None """
# GH # 17276
tm.assert_frame_equal(self.frame.clip(np.nan), self.frame)
tm.assert_frame_equal(self.frame.clip(upper=[1, 2, np.nan]),
self.frame)
tm.assert_frame_equal(self.frame.clip(lower=[1, np.nan, 3]),
self.frame)
tm.assert_frame_equal(self.frame.clip(upper=np.nan, lower=np.nan),
self.frame)

# Matrix-like

Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/series/test_analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,17 @@ def test_clip_types_and_nulls(self):
assert list(isna(s)) == list(isna(l))
assert list(isna(s)) == list(isna(u))

def test_clip_with_na_args(self):
"""Should process np.nan argument as None """
# GH # 17276
s = Series([1, 2, 3])

assert_series_equal(s.clip(np.nan), Series([1, 2, 3]))
assert_series_equal(s.clip(upper=[1, 1, np.nan]), Series([1, 2, 3]))
assert_series_equal(s.clip(lower=[1, np.nan, 1]), Series([1, 2, 3]))
assert_series_equal(s.clip(upper=np.nan, lower=np.nan),
Series([1, 2, 3]))

def test_clip_against_series(self):
# GH #6966

Expand Down

0 comments on commit 910207f

Please sign in to comment.