Skip to content

Commit ce1d2bf

Browse files
committed
BUG: Fix Series.sort_values descending & mergesort
The solution uses the `nargsort` function, which was created to handle in one place the intricacies of sorting. Not adapting Series.sort_values to use may have been an oversight. closes #28697
1 parent 3daceae commit ce1d2bf

File tree

3 files changed

+17
-34
lines changed

3 files changed

+17
-34
lines changed

doc/source/whatsnew/v1.0.0.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,8 @@ Other
445445
- Fix corrupted error message when calling ``pandas.libs._json.encode()`` on a 0d array (:issue:`18878`)
446446
- Fix :class:`AbstractHolidayCalendar` to return correct results for
447447
years after 2030 (now goes up to 2200) (:issue:`27790`)
448+
- Bug in :meth:`Series.sort_values` when ``ascending`` is set to ``False`` and
449+
``kind`` is set to ``mergesort`` (:issue:`28697`)
448450

449451

450452
.. _whatsnew_1000.contributors:

pandas/core/series.py

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
from pandas.core.indexes.timedeltas import TimedeltaIndex
7676
from pandas.core.indexing import check_bool_indexer
7777
from pandas.core.internals import SingleBlockManager
78+
from pandas.core.sorting import nargsort
7879
from pandas.core.strings import StringMethods
7980
from pandas.core.tools.datetimes import to_datetime
8081

@@ -3077,26 +3078,6 @@ def sort_values(
30773078
"sort in-place you must create a copy"
30783079
)
30793080

3080-
def _try_kind_sort(arr):
3081-
# easier to ask forgiveness than permission
3082-
try:
3083-
# if kind==mergesort, it can fail for object dtype
3084-
return arr.argsort(kind=kind)
3085-
except TypeError:
3086-
# stable sort not available for object dtype
3087-
# uses the argsort default quicksort
3088-
return arr.argsort(kind="quicksort")
3089-
3090-
arr = self._values
3091-
sortedIdx = np.empty(len(self), dtype=np.int32)
3092-
3093-
bad = isna(arr)
3094-
3095-
good = ~bad
3096-
idx = ibase.default_index(len(self))
3097-
3098-
argsorted = _try_kind_sort(arr[good])
3099-
31003081
if is_list_like(ascending):
31013082
if len(ascending) != 1:
31023083
raise ValueError(
@@ -3108,21 +3089,10 @@ def _try_kind_sort(arr):
31083089
if not is_bool(ascending):
31093090
raise ValueError("ascending must be boolean")
31103091

3111-
if not ascending:
3112-
argsorted = argsorted[::-1]
3113-
3114-
if na_position == "last":
3115-
n = good.sum()
3116-
sortedIdx[:n] = idx[good][argsorted]
3117-
sortedIdx[n:] = idx[bad]
3118-
elif na_position == "first":
3119-
n = bad.sum()
3120-
sortedIdx[n:] = idx[good][argsorted]
3121-
sortedIdx[:n] = idx[bad]
3122-
else:
3123-
raise ValueError("invalid na_position: {!r}".format(na_position))
3092+
arr = self._values
3093+
indexer = nargsort(arr, kind=kind, ascending=ascending, na_position=na_position)
31243094

3125-
result = self._constructor(arr[sortedIdx], index=self.index[sortedIdx])
3095+
result = self._constructor(arr[indexer], index=self.index[indexer])
31263096

31273097
if inplace:
31283098
self._update_inplace(result)

pandas/tests/series/test_sorting.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,17 @@ def test_sort_values(self, datetime_series):
8686
with pytest.raises(ValueError, match=msg):
8787
s.sort_values(inplace=True)
8888

89+
def test_sort_values_mergesort(self):
90+
ser = Series([1, 2, 1, 3], ["first", "b", "second", "c"])
91+
expected = Series([1, 1, 2, 3], ["first", "second", "b", "c"])
92+
result = ser.sort_values(kind="mergesort")
93+
tm.assert_series_equal(expected, result)
94+
95+
# ascending=False is not just a reverse of ascending=True
96+
expected = Series([3, 2, 1, 1], ["c", "b", "first", "second"])
97+
result = ser.sort_values(ascending=False, kind="mergesort")
98+
tm.assert_series_equal(expected, result)
99+
89100
def test_sort_index(self, datetime_series):
90101
rindex = list(datetime_series.index)
91102
random.shuffle(rindex)

0 commit comments

Comments
 (0)