diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst index 4007ecd5a9412..d834daf88197d 100644 --- a/doc/source/whatsnew/v1.0.0.rst +++ b/doc/source/whatsnew/v1.0.0.rst @@ -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: diff --git a/pandas/core/series.py b/pandas/core/series.py index 3e9d3d5c04559..47044cac32370 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -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 @@ -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( @@ -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) diff --git a/pandas/tests/series/test_sorting.py b/pandas/tests/series/test_sorting.py index 192b57d2a9007..fffd8ced6a27b 100644 --- a/pandas/tests/series/test_sorting.py +++ b/pandas/tests/series/test_sorting.py @@ -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)