Skip to content

BUG: Fix Series.sort_values descending & mergesort #28698

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,8 @@ Other
- Fix corrupted error message when calling ``pandas.libs._json.encode()`` on a 0d array (:issue:`18878`)
- Fix :class:`AbstractHolidayCalendar` to return correct results for
years after 2030 (now goes up to 2200) (:issue:`27790`)
- Bug in :meth:`Series.sort_values` when ``ascending`` is set to ``False`` and
``kind`` is set to ``mergesort`` (:issue:`28697`)


.. _whatsnew_1000.contributors:
Expand Down
38 changes: 4 additions & 34 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
from pandas.core.indexes.timedeltas import TimedeltaIndex
from pandas.core.indexing import check_bool_indexer
from pandas.core.internals import SingleBlockManager
from pandas.core.sorting import nargsort
from pandas.core.strings import StringMethods
from pandas.core.tools.datetimes import to_datetime

Expand Down Expand Up @@ -3077,26 +3078,6 @@ def sort_values(
"sort in-place you must create a copy"
)

def _try_kind_sort(arr):
# easier to ask forgiveness than permission
try:
# if kind==mergesort, it can fail for object dtype
return arr.argsort(kind=kind)
except TypeError:
# stable sort not available for object dtype
# uses the argsort default quicksort
return arr.argsort(kind="quicksort")

arr = self._values
sortedIdx = np.empty(len(self), dtype=np.int32)

bad = isna(arr)

good = ~bad
idx = ibase.default_index(len(self))

argsorted = _try_kind_sort(arr[good])

if is_list_like(ascending):
if len(ascending) != 1:
raise ValueError(
Expand All @@ -3108,21 +3089,10 @@ def _try_kind_sort(arr):
if not is_bool(ascending):
raise ValueError("ascending must be boolean")

if not ascending:
argsorted = argsorted[::-1]

if na_position == "last":
n = good.sum()
sortedIdx[:n] = idx[good][argsorted]
sortedIdx[n:] = idx[bad]
elif na_position == "first":
n = bad.sum()
sortedIdx[n:] = idx[good][argsorted]
sortedIdx[:n] = idx[bad]
else:
raise ValueError("invalid na_position: {!r}".format(na_position))
arr = self._values
indexer = nargsort(arr, kind=kind, ascending=ascending, na_position=na_position)

result = self._constructor(arr[sortedIdx], index=self.index[sortedIdx])
result = self._constructor(arr[indexer], index=self.index[indexer])

if inplace:
self._update_inplace(result)
Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/series/test_sorting.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ def test_sort_values(self, datetime_series):
with pytest.raises(ValueError, match=msg):
s.sort_values(inplace=True)

def test_sort_values_mergesort(self):
ser = Series([1, 2, 1, 3], ["first", "b", "second", "c"])
expected = Series([1, 1, 2, 3], ["first", "second", "b", "c"])
result = ser.sort_values(kind="mergesort")
tm.assert_series_equal(expected, result)

# ascending=False is not just a reverse of ascending=True
expected = Series([3, 2, 1, 1], ["c", "b", "first", "second"])
result = ser.sort_values(ascending=False, kind="mergesort")
tm.assert_series_equal(expected, result)

def test_sort_index(self, datetime_series):
rindex = list(datetime_series.index)
random.shuffle(rindex)
Expand Down