Skip to content

Commit 90f3797

Browse files
Backport PR pandas-dev#39451: BUG: raise when sort_index with ascending=None (pandas-dev#40083)
Co-authored-by: Maxim Ivanov <[email protected]>
1 parent 3667165 commit 90f3797

File tree

8 files changed

+97
-13
lines changed

8 files changed

+97
-13
lines changed

doc/source/whatsnew/v1.2.3.rst

+5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ Fixed regressions
1919
- Fixed regression in nullable integer unary ops propagating mask on assignment (:issue:`39943`)
2020
- Fixed regression in :meth:`DataFrame.__setitem__` not aligning :class:`DataFrame` on right-hand side for boolean indexer (:issue:`39931`)
2121
- Fixed regression in :meth:`~DataFrame.to_json` failing to use ``compression`` with URL-like paths that are internally opened in binary mode or with user-provided file objects that are opened in binary mode (:issue:`39985`)
22+
- Fixed regression in :meth:`~Series.sort_index` and :meth:`~DataFrame.sort_index`,
23+
which exited with an ungraceful error when having kwarg ``ascending=None`` passed (:issue:`39434`).
24+
Passing ``ascending=None`` is still considered invalid,
25+
and the new error message suggests a proper usage
26+
(``ascending`` must be a boolean or a list-like boolean).
2227

2328
.. ---------------------------------------------------------------------------
2429

pandas/core/frame.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -5482,7 +5482,7 @@ def sort_index(
54825482
self,
54835483
axis=0,
54845484
level=None,
5485-
ascending: bool = True,
5485+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
54865486
inplace: bool = False,
54875487
kind: str = "quicksort",
54885488
na_position: str = "last",
@@ -5503,7 +5503,7 @@ def sort_index(
55035503
and 1 identifies the columns.
55045504
level : int or level name or list of ints or list of level names
55055505
If not None, sort on values in specified index level(s).
5506-
ascending : bool or list of bools, default True
5506+
ascending : bool or list-like of bools, default True
55075507
Sort ascending vs. descending. When the index is a MultiIndex the
55085508
sort direction can be controlled for each level individually.
55095509
inplace : bool, default False

pandas/core/generic.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
from pandas.errors import AbstractMethodError, InvalidIndexError
5757
from pandas.util._decorators import doc, rewrite_axis_style_signature
5858
from pandas.util._validators import (
59+
validate_ascending,
5960
validate_bool_kwarg,
6061
validate_fillna_kwargs,
6162
validate_percentile,
@@ -4518,7 +4519,7 @@ def sort_index(
45184519
self,
45194520
axis=0,
45204521
level=None,
4521-
ascending: bool_t = True,
4522+
ascending: Union[Union[bool_t, int], Sequence[Union[bool_t, int]]] = True,
45224523
inplace: bool_t = False,
45234524
kind: str = "quicksort",
45244525
na_position: str = "last",
@@ -4529,6 +4530,8 @@ def sort_index(
45294530

45304531
inplace = validate_bool_kwarg(inplace, "inplace")
45314532
axis = self._get_axis_number(axis)
4533+
ascending = validate_ascending(ascending)
4534+
45324535
target = self._get_axis(axis)
45334536

45344537
indexer = get_indexer_indexer(

pandas/core/series.py

+8-5
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
Iterable,
1313
List,
1414
Optional,
15+
Sequence,
1516
Tuple,
1617
Type,
1718
Union,
19+
cast,
1820
)
1921
import warnings
2022

@@ -3065,7 +3067,7 @@ def update(self, other) -> None:
30653067
def sort_values(
30663068
self,
30673069
axis=0,
3068-
ascending=True,
3070+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
30693071
inplace: bool = False,
30703072
kind: str = "quicksort",
30713073
na_position: str = "last",
@@ -3083,7 +3085,7 @@ def sort_values(
30833085
axis : {0 or 'index'}, default 0
30843086
Axis to direct sorting. The value 'index' is accepted for
30853087
compatibility with DataFrame.sort_values.
3086-
ascending : bool, default True
3088+
ascending : bool or list of bools, default True
30873089
If True, sort values in ascending order, otherwise descending.
30883090
inplace : bool, default False
30893091
If True, perform operation in-place.
@@ -3243,6 +3245,7 @@ def sort_values(
32433245
)
32443246

32453247
if is_list_like(ascending):
3248+
ascending = cast(Sequence[Union[bool, int]], ascending)
32463249
if len(ascending) != 1:
32473250
raise ValueError(
32483251
f"Length of ascending ({len(ascending)}) must be 1 for Series"
@@ -3257,7 +3260,7 @@ def sort_values(
32573260

32583261
# GH 35922. Make sorting stable by leveraging nargsort
32593262
values_to_sort = ensure_key_mapped(self, key)._values if key else self._values
3260-
sorted_index = nargsort(values_to_sort, kind, ascending, na_position)
3263+
sorted_index = nargsort(values_to_sort, kind, bool(ascending), na_position)
32613264

32623265
result = self._constructor(
32633266
self._values[sorted_index], index=self.index[sorted_index]
@@ -3275,7 +3278,7 @@ def sort_index(
32753278
self,
32763279
axis=0,
32773280
level=None,
3278-
ascending: bool = True,
3281+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
32793282
inplace: bool = False,
32803283
kind: str = "quicksort",
32813284
na_position: str = "last",
@@ -3295,7 +3298,7 @@ def sort_index(
32953298
Axis to direct sorting. This can only be 0 for Series.
32963299
level : int, optional
32973300
If not None, sort on values in specified index level(s).
3298-
ascending : bool or list of bools, default True
3301+
ascending : bool or list-like of bools, default True
32993302
Sort ascending vs. descending. When the index is a MultiIndex the
33003303
sort direction can be controlled for each level individually.
33013304
inplace : bool, default False

pandas/core/sorting.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
Iterable,
99
List,
1010
Optional,
11+
Sequence,
1112
Tuple,
1213
Union,
1314
)
@@ -39,7 +40,7 @@
3940
def get_indexer_indexer(
4041
target: "Index",
4142
level: Union[str, int, List[str], List[int]],
42-
ascending: bool,
43+
ascending: Union[Sequence[Union[bool, int]], Union[bool, int]],
4344
kind: str,
4445
na_position: str,
4546
sort_remaining: bool,

pandas/tests/frame/methods/test_sort_index.py

+17
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,23 @@ def test_sort_index_with_categories(self, categories):
765765
)
766766
tm.assert_frame_equal(result, expected)
767767

768+
@pytest.mark.parametrize(
769+
"ascending",
770+
[
771+
None,
772+
[True, None],
773+
[False, "True"],
774+
],
775+
)
776+
def test_sort_index_ascending_bad_value_raises(self, ascending):
777+
# GH 39434
778+
df = DataFrame(np.arange(64))
779+
length = len(df.index)
780+
df.index = [(i - length / 2) % length for i in range(length)]
781+
match = 'For argument "ascending" expected type bool'
782+
with pytest.raises(ValueError, match=match):
783+
df.sort_index(axis=0, ascending=ascending, na_position="first")
784+
768785

769786
class TestDataFrameSortIndexKey:
770787
def test_sort_multi_index_key(self):

pandas/tests/series/methods/test_sort_index.py

+14
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,20 @@ def test_sort_index_ascending_list(self):
199199
expected = ser.iloc[[0, 4, 1, 5, 2, 6, 3, 7]]
200200
tm.assert_series_equal(result, expected)
201201

202+
@pytest.mark.parametrize(
203+
"ascending",
204+
[
205+
None,
206+
(True, None),
207+
(False, "True"),
208+
],
209+
)
210+
def test_sort_index_ascending_bad_value_raises(self, ascending):
211+
ser = Series(range(10), index=[0, 3, 2, 1, 4, 5, 7, 6, 8, 9])
212+
match = 'For argument "ascending" expected type bool'
213+
with pytest.raises(ValueError, match=match):
214+
ser.sort_index(ascending=ascending)
215+
202216

203217
class TestSeriesSortIndexKey:
204218
def test_sort_index_multiindex_key(self):

pandas/util/_validators.py

+45-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Module that contains many useful utilities
33
for validating data or function arguments
44
"""
5-
from typing import Iterable, Union
5+
from typing import Iterable, Sequence, Union
66
import warnings
77

88
import numpy as np
@@ -205,9 +205,39 @@ def validate_args_and_kwargs(fname, args, kwargs, max_fname_arg_count, compat_ar
205205
validate_kwargs(fname, kwargs, compat_args)
206206

207207

208-
def validate_bool_kwarg(value, arg_name):
209-
""" Ensures that argument passed in arg_name is of type bool. """
210-
if not (is_bool(value) or value is None):
208+
def validate_bool_kwarg(value, arg_name, none_allowed=True, int_allowed=False):
209+
"""
210+
Ensure that argument passed in arg_name can be interpreted as boolean.
211+
212+
Parameters
213+
----------
214+
value : bool
215+
Value to be validated.
216+
arg_name : str
217+
Name of the argument. To be reflected in the error message.
218+
none_allowed : bool, default True
219+
Whether to consider None to be a valid boolean.
220+
int_allowed : bool, default False
221+
Whether to consider integer value to be a valid boolean.
222+
223+
Returns
224+
-------
225+
value
226+
The same value as input.
227+
228+
Raises
229+
------
230+
ValueError
231+
If the value is not a valid boolean.
232+
"""
233+
good_value = is_bool(value)
234+
if none_allowed:
235+
good_value = good_value or value is None
236+
237+
if int_allowed:
238+
good_value = good_value or isinstance(value, int)
239+
240+
if not good_value:
211241
raise ValueError(
212242
f'For argument "{arg_name}" expected type bool, received '
213243
f"type {type(value).__name__}."
@@ -381,3 +411,14 @@ def validate_percentile(q: Union[float, Iterable[float]]) -> np.ndarray:
381411
if not all(0 <= qs <= 1 for qs in q_arr):
382412
raise ValueError(msg.format(q_arr / 100.0))
383413
return q_arr
414+
415+
416+
def validate_ascending(
417+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
418+
):
419+
"""Validate ``ascending`` kwargs for ``sort_index`` method."""
420+
kwargs = {"none_allowed": False, "int_allowed": True}
421+
if not isinstance(ascending, (list, tuple)):
422+
return validate_bool_kwarg(ascending, "ascending", **kwargs)
423+
424+
return [validate_bool_kwarg(item, "ascending", **kwargs) for item in ascending]

0 commit comments

Comments
 (0)