Skip to content

PERF: DataFrame.clip / Series.clip #51472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Feb 22, 2023
15 changes: 15 additions & 0 deletions asv_bench/benchmarks/frame_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@
from .pandas_vb_common import tm


class Clip:
params = [
["float64", "Float64", "float64[pyarrow]"],
]
param_names = ["dtype"]

def setup(self, dtype):
data = np.random.randn(100_000, 10)
df = DataFrame(data, dtype=dtype)
self.df = df

def time_clip(self, dtype):
self.df.clip(-1.0, 1.0)


class GetNumericData:
def setup(self):
self.df = DataFrame(np.random.randn(10000, 25))
Expand Down
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,7 @@ Performance improvements
- Performance improvement in :func:`merge` and :meth:`DataFrame.join` when joining on a sorted :class:`MultiIndex` (:issue:`48504`)
- Performance improvement in :func:`to_datetime` when parsing strings with timezone offsets (:issue:`50107`)
- Performance improvement in :meth:`DataFrame.loc` and :meth:`Series.loc` for tuple-based indexing of a :class:`MultiIndex` (:issue:`48384`)
- Performance improvement in :meth:`DataFrame.clip` and :meth:`Series.clip` (:issue:`51472`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last comment. Can you move to 2.1?

- Performance improvement for :meth:`Series.replace` with categorical dtype (:issue:`49404`)
- Performance improvement for :meth:`MultiIndex.unique` (:issue:`48335`)
- Performance improvement for indexing operations with nullable and arrow dtypes (:issue:`49420`, :issue:`51316`)
Expand Down
27 changes: 16 additions & 11 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -7995,24 +7995,29 @@ def _clip_with_scalar(self, lower, upper, inplace: bool_t = False):
):
raise ValueError("Cannot use an NA value as a clip threshold")

result = self
mask = isna(self._values)
mgr = self._mgr

with np.errstate(all="ignore"):
if inplace:
if upper is not None:
subset = self <= upper
result = result.where(subset, upper, axis=None, inplace=False)
cond = self > upper
mgr = mgr.putmask(mask=cond, new=upper, align=False)
if lower is not None:
subset = self >= lower
result = result.where(subset, lower, axis=None, inplace=False)

if np.any(mask):
result[mask] = np.nan
cond = self < lower
mgr = mgr.putmask(mask=cond, new=lower, align=False)
else:
mask = isna(self)
if upper is not None:
cond = mask | (self <= upper)
mgr = mgr.where(other=upper, cond=cond, align=False)
if lower is not None:
cond = mask | (self >= lower)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use > or >= in both cases? Also why don’t we need a mask in the inplace case?

Copy link
Member Author

@lukemanley lukemanley Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition in putmask identifies the values to be updated so we look for values outside the threshold (e.g. ">")

The condition in where identifies the values to be left as is which are NA values (mask) and anything within the threshold (e.g. "<=")

So I think the difference is consistent with the meaning of those different methods. Using <= and >= in putmask would actually turn no-ops at the boundary into unnecessary ops I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, makes sense. Did not consider that, could you add a really short comment to that effect?

mgr = mgr.where(other=lower, cond=cond, align=False)

result = self._constructor(mgr)
if inplace:
return self._update_inplace(result)
else:
return result
return result.__finalize__(self)

@final
def _clip_with_one_bound(self, threshold, method, axis, inplace):
Expand Down
14 changes: 6 additions & 8 deletions pandas/tests/copy_view/test_clip.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ def test_clip_inplace_reference(using_copy_on_write):
view = df[:]
df.clip(lower=2, inplace=True)

# Clip not actually inplace right now but could be
assert not np.shares_memory(get_array(df, "a"), arr_a)

if using_copy_on_write:
assert not np.shares_memory(get_array(df, "a"), arr_a)
assert df._mgr._has_no_reference(0)
assert view._mgr._has_no_reference(0)
tm.assert_frame_equal(df_copy, view)
else:
assert np.shares_memory(get_array(df, "a"), arr_a)


def test_clip_inplace_reference_no_op(using_copy_on_write):
Expand All @@ -28,22 +28,20 @@ def test_clip_inplace_reference_no_op(using_copy_on_write):
view = df[:]
df.clip(lower=0, inplace=True)

assert np.shares_memory(get_array(df, "a"), arr_a)

if using_copy_on_write:
assert np.shares_memory(get_array(df, "a"), arr_a)
assert not df._mgr._has_no_reference(0)
assert not view._mgr._has_no_reference(0)
tm.assert_frame_equal(df_copy, view)
else:
assert not np.shares_memory(get_array(df, "a"), arr_a)


def test_clip_inplace(using_copy_on_write):
df = DataFrame({"a": [1.5, 2, 3]})
arr_a = get_array(df, "a")
df.clip(lower=2, inplace=True)

# Clip not actually inplace right now but could be
assert not np.shares_memory(get_array(df, "a"), arr_a)
assert np.shares_memory(get_array(df, "a"), arr_a)

if using_copy_on_write:
assert df._mgr._has_no_reference(0)
Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/frame/methods/test_clip.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,10 @@ def test_clip_with_na_args(self, float_frame):
result = df.clip(lower=t, axis=0)
expected = DataFrame({"col_0": [9, -3, 0, 6, 5], "col_1": [2, -4, 6, 8, 3]})
tm.assert_frame_equal(result, expected)

def test_clip_int_data_with_float_bound(self):
# GH51472
df = DataFrame({"a": [1, 2, 3]})
result = df.clip(lower=1.5)
expected = DataFrame({"a": [1.5, 2.0, 3.0]})
tm.assert_frame_equal(result, expected)