Skip to content

ENH: Add lazy copy for sort_values #50643

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 11 commits into from
Jan 13, 2023
Merged
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 @@ -953,6 +953,7 @@ Indexing
- Bug in :meth:`DataFrame.__setitem__` raising when indexer is a :class:`DataFrame` with ``boolean`` dtype (:issue:`47125`)
- Bug in :meth:`DataFrame.reindex` filling with wrong values when indexing columns and index for ``uint`` dtypes (:issue:`48184`)
- Bug in :meth:`DataFrame.loc` when setting :class:`DataFrame` with different dtypes coercing values to single dtype (:issue:`50467`)
- Bug in :meth:`DataFrame.sort_values` where ``None`` was not returned when ``by`` is empty list and ``inplace=True`` (:issue:`50643`)
- Bug in :meth:`DataFrame.loc` coercing dtypes when setting values with a list indexer (:issue:`49159`)
- Bug in :meth:`Series.loc` raising error for out of bounds end of slice indexer (:issue:`50161`)
- Bug in :meth:`DataFrame.loc` raising ``ValueError`` with ``bool`` indexer and :class:`MultiIndex` (:issue:`47687`)
Expand Down
12 changes: 11 additions & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from pandas._libs.hashtable import duplicated
from pandas._libs.lib import (
NoDefault,
array_equal_fast,
no_default,
)
from pandas._typing import (
Expand Down Expand Up @@ -6695,7 +6696,16 @@ def sort_values(
k, kind=kind, ascending=ascending, na_position=na_position, key=key
)
else:
return self.copy()
if inplace:
return self._update_inplace(self)
else:
return self.copy(deep=None)

if array_equal_fast(indexer, np.arange(0, len(indexer), dtype=indexer.dtype)):
if inplace:
return self._update_inplace(self)
else:
return self.copy(deep=None)

new_data = self._mgr.take(
indexer, axis=self._get_block_manager_axis(axis), verify=False
Expand Down
7 changes: 7 additions & 0 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -3559,6 +3559,13 @@ def sort_values(
values_to_sort = ensure_key_mapped(self, key)._values if key else self._values
sorted_index = nargsort(values_to_sort, kind, bool(ascending), na_position)

if array_equal_fast(
sorted_index, np.arange(0, len(sorted_index), dtype=sorted_index.dtype)
):
if inplace:
return self._update_inplace(self)
return self.copy(deep=None)

result = self._constructor(
self._values[sorted_index], index=self.index[sorted_index]
)
Expand Down
39 changes: 39 additions & 0 deletions pandas/tests/copy_view/test_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,45 @@ def test_sort_index(using_copy_on_write):
tm.assert_series_equal(ser, ser_orig)


@pytest.mark.parametrize(
"obj, kwargs",
[(Series([1, 2, 3], name="a"), {}), (DataFrame({"a": [1, 2, 3]}), {"by": "a"})],
)
def test_sort_values(using_copy_on_write, obj, kwargs):
obj_orig = obj.copy()
obj2 = obj.sort_values(**kwargs)

if using_copy_on_write:
assert np.shares_memory(get_array(obj2, "a"), get_array(obj, "a"))
else:
assert not np.shares_memory(get_array(obj2, "a"), get_array(obj, "a"))

# mutating df triggers a copy-on-write for the column / block
obj2.iloc[0] = 0
assert not np.shares_memory(get_array(obj2, "a"), get_array(obj, "a"))
tm.assert_equal(obj, obj_orig)


@pytest.mark.parametrize(
"obj, kwargs",
[(Series([1, 2, 3], name="a"), {}), (DataFrame({"a": [1, 2, 3]}), {"by": "a"})],
)
def test_sort_values_inplace(using_copy_on_write, obj, kwargs, using_array_manager):
obj_orig = obj.copy()
view = obj[:]
obj.sort_values(inplace=True, **kwargs)

assert np.shares_memory(get_array(obj, "a"), get_array(view, "a"))

# mutating obj triggers a copy-on-write for the column / block
obj.iloc[0] = 0
if using_copy_on_write:
assert not np.shares_memory(get_array(obj, "a"), get_array(view, "a"))
tm.assert_equal(view, obj_orig)
else:
assert np.shares_memory(get_array(obj, "a"), get_array(view, "a"))


def test_reorder_levels(using_copy_on_write):
index = MultiIndex.from_tuples(
[(1, 1), (1, 2), (2, 1), (2, 2)], names=["one", "two"]
Expand Down
8 changes: 8 additions & 0 deletions pandas/tests/frame/methods/test_sort_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,14 @@ def test_sort_values_reshaping(self):

tm.assert_frame_equal(df, expected)

def test_sort_values_no_by_inplace(self):
# GH#50643
df = DataFrame({"a": [1, 2, 3]})
expected = df.copy()
result = df.sort_values(by=[], inplace=True)
tm.assert_frame_equal(df, expected)
assert result is None


class TestDataFrameSortKey: # test key sorting (issue 27237)
def test_sort_values_inplace_key(self, sort_by_key):
Expand Down