Skip to content

Commit a84e1c0

Browse files
BUG: Series.sort_values not raising ValueError for ascending-incompatible value (pandas-dev#41634)
1 parent 3677a9e commit a84e1c0

File tree

3 files changed

+24
-6
lines changed

3 files changed

+24
-6
lines changed

doc/source/whatsnew/v1.4.0.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ Indexing
225225
- Bug in indexing on a :class:`Series` or :class:`DataFrame` with a :class:`DatetimeIndex` when passing a string, the return type depended on whether the index was monotonic (:issue:`24892`)
226226
- Bug in indexing on a :class:`MultiIndex` failing to drop scalar levels when the indexer is a tuple containing a datetime-like string (:issue:`42476`)
227227
- Bug in :meth:`Dataframe.sort_values` when passing a ascending-incompatible value, failed to raise ``ValueError`` (:issue:`41634`)
228-
228+
- Bug in :meth:`Series.sort_values` when passing a ascending-incompatible value, failed to raise ``ValueError`` (:issue:`41634`)
229229

230230
Missing
231231
^^^^^^^

pandas/core/series.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
doc,
5858
)
5959
from pandas.util._validators import (
60+
validate_ascending,
6061
validate_bool_kwarg,
6162
validate_percentile,
6263
)
@@ -69,7 +70,6 @@
6970
)
7071
from pandas.core.dtypes.common import (
7172
ensure_platform_int,
72-
is_bool,
7373
is_dict_like,
7474
is_integer,
7575
is_iterator,
@@ -3437,8 +3437,7 @@ def sort_values(
34373437
)
34383438
ascending = ascending[0]
34393439

3440-
if not is_bool(ascending):
3441-
raise ValueError("ascending must be boolean")
3440+
ascending = validate_ascending(ascending)
34423441

34433442
if na_position not in ["first", "last"]:
34443443
raise ValueError(f"invalid na_position: {na_position}")

pandas/tests/series/methods/test_sort_values.py

+21-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def test_sort_values(self, datetime_series):
5151
expected = ts.sort_values(ascending=False, na_position="first")
5252
tm.assert_series_equal(expected, ordered)
5353

54-
msg = "ascending must be boolean"
54+
msg = 'For argument "ascending" expected type bool, received type NoneType.'
5555
with pytest.raises(ValueError, match=msg):
5656
ts.sort_values(ascending=None)
5757
msg = r"Length of ascending \(0\) must be 1 for Series"
@@ -63,7 +63,7 @@ def test_sort_values(self, datetime_series):
6363
msg = r"Length of ascending \(2\) must be 1 for Series"
6464
with pytest.raises(ValueError, match=msg):
6565
ts.sort_values(ascending=[False, False])
66-
msg = "ascending must be boolean"
66+
msg = 'For argument "ascending" expected type bool, received type str.'
6767
with pytest.raises(ValueError, match=msg):
6868
ts.sort_values(ascending="foobar")
6969

@@ -206,6 +206,25 @@ def test_mergesort_decending_stability(self):
206206
expected = Series([3, 2, 1, 1], ["c", "b", "first", "second"])
207207
tm.assert_series_equal(result, expected)
208208

209+
def test_sort_values_validate_ascending(self):
210+
# GH41634
211+
ser = Series([23, 7, 21])
212+
expected = np.sort(ser.values)[::-1]
213+
214+
msg = 'For argument "ascending" expected type bool, received type str.'
215+
with pytest.raises(ValueError, match=msg):
216+
ser.sort_values(ascending="False")
217+
218+
sorted_ser = ser.sort_values(ascending=False)
219+
tm.assert_numpy_array_equal(sorted_ser.values, expected)
220+
221+
sorted_ser = ser.sort_values(ascending=0)
222+
tm.assert_numpy_array_equal(sorted_ser.values, expected)
223+
224+
expected = expected[::-1]
225+
sorted_ser = ser.sort_values(ascending=1)
226+
tm.assert_numpy_array_equal(sorted_ser.values, expected)
227+
209228

210229
class TestSeriesSortingKey:
211230
def test_sort_values_key(self):

0 commit comments

Comments
 (0)