Skip to content

Commit 78371cc

Browse files
authored
BUG: raise when sort_index with ascending=None (#39451)
1 parent ab687ae commit 78371cc

File tree

8 files changed

+97
-13
lines changed

8 files changed

+97
-13
lines changed

doc/source/whatsnew/v1.2.3.rst

+5-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +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-
-
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).
2327

2428
.. ---------------------------------------------------------------------------
2529

pandas/core/frame.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -5625,7 +5625,7 @@ def sort_index(
56255625
self,
56265626
axis: Axis = 0,
56275627
level: Optional[Level] = None,
5628-
ascending: bool = True,
5628+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
56295629
inplace: bool = False,
56305630
kind: str = "quicksort",
56315631
na_position: str = "last",
@@ -5646,7 +5646,7 @@ def sort_index(
56465646
and 1 identifies the columns.
56475647
level : int or level name or list of ints or list of level names
56485648
If not None, sort on values in specified index level(s).
5649-
ascending : bool or list of bools, default True
5649+
ascending : bool or list-like of bools, default True
56505650
Sort ascending vs. descending. When the index is a MultiIndex the
56515651
sort direction can be controlled for each level individually.
56525652
inplace : bool, default False

pandas/core/generic.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
rewrite_axis_style_signature,
7272
)
7373
from pandas.util._validators import (
74+
validate_ascending,
7475
validate_bool_kwarg,
7576
validate_fillna_kwargs,
7677
)
@@ -4548,7 +4549,7 @@ def sort_index(
45484549
self,
45494550
axis=0,
45504551
level=None,
4551-
ascending: bool_t = True,
4552+
ascending: Union[Union[bool_t, int], Sequence[Union[bool_t, int]]] = True,
45524553
inplace: bool_t = False,
45534554
kind: str = "quicksort",
45544555
na_position: str = "last",
@@ -4559,6 +4560,8 @@ def sort_index(
45594560

45604561
inplace = validate_bool_kwarg(inplace, "inplace")
45614562
axis = self._get_axis_number(axis)
4563+
ascending = validate_ascending(ascending)
4564+
45624565
target = self._get_axis(axis)
45634566

45644567
indexer = get_indexer_indexer(

pandas/core/series.py

+8-5
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
Iterable,
1616
List,
1717
Optional,
18+
Sequence,
1819
Tuple,
1920
Type,
2021
Union,
22+
cast,
2123
)
2224
import warnings
2325

@@ -3099,7 +3101,7 @@ def update(self, other) -> None:
30993101
def sort_values(
31003102
self,
31013103
axis=0,
3102-
ascending=True,
3104+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
31033105
inplace: bool = False,
31043106
kind: str = "quicksort",
31053107
na_position: str = "last",
@@ -3117,7 +3119,7 @@ def sort_values(
31173119
axis : {0 or 'index'}, default 0
31183120
Axis to direct sorting. The value 'index' is accepted for
31193121
compatibility with DataFrame.sort_values.
3120-
ascending : bool, default True
3122+
ascending : bool or list of bools, default True
31213123
If True, sort values in ascending order, otherwise descending.
31223124
inplace : bool, default False
31233125
If True, perform operation in-place.
@@ -3277,6 +3279,7 @@ def sort_values(
32773279
)
32783280

32793281
if is_list_like(ascending):
3282+
ascending = cast(Sequence[Union[bool, int]], ascending)
32803283
if len(ascending) != 1:
32813284
raise ValueError(
32823285
f"Length of ascending ({len(ascending)}) must be 1 for Series"
@@ -3291,7 +3294,7 @@ def sort_values(
32913294

32923295
# GH 35922. Make sorting stable by leveraging nargsort
32933296
values_to_sort = ensure_key_mapped(self, key)._values if key else self._values
3294-
sorted_index = nargsort(values_to_sort, kind, ascending, na_position)
3297+
sorted_index = nargsort(values_to_sort, kind, bool(ascending), na_position)
32953298

32963299
result = self._constructor(
32973300
self._values[sorted_index], index=self.index[sorted_index]
@@ -3309,7 +3312,7 @@ def sort_index(
33093312
self,
33103313
axis=0,
33113314
level=None,
3312-
ascending: bool = True,
3315+
ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True,
33133316
inplace: bool = False,
33143317
kind: str = "quicksort",
33153318
na_position: str = "last",
@@ -3329,7 +3332,7 @@ def sort_index(
33293332
Axis to direct sorting. This can only be 0 for Series.
33303333
level : int, optional
33313334
If not None, sort on values in specified index level(s).
3332-
ascending : bool or list of bools, default True
3335+
ascending : bool or list-like of bools, default True
33333336
Sort ascending vs. descending. When the index is a MultiIndex the
33343337
sort direction can be controlled for each level individually.
33353338
inplace : bool, default False

pandas/core/sorting.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
Iterable,
1111
List,
1212
Optional,
13+
Sequence,
1314
Tuple,
1415
Union,
1516
)
@@ -45,7 +46,7 @@
4546
def get_indexer_indexer(
4647
target: Index,
4748
level: Union[str, int, List[str], List[int]],
48-
ascending: bool,
49+
ascending: Union[Sequence[Union[bool, int]], Union[bool, int]],
4950
kind: str,
5051
na_position: str,
5152
sort_remaining: bool,

pandas/tests/frame/methods/test_sort_index.py

+17
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,23 @@ def test_sort_index_with_categories(self, categories):
761761
)
762762
tm.assert_frame_equal(result, expected)
763763

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

765782
class TestDataFrameSortIndexKey:
766783
def test_sort_multi_index_key(self):

pandas/tests/series/methods/test_sort_index.py

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

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

207221
class TestSeriesSortIndexKey:
208222
def test_sort_index_multiindex_key(self):

pandas/util/_validators.py

+45-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""
55
from typing import (
66
Iterable,
7+
Sequence,
78
Union,
89
)
910
import warnings
@@ -208,9 +209,39 @@ def validate_args_and_kwargs(fname, args, kwargs, max_fname_arg_count, compat_ar
208209
validate_kwargs(fname, kwargs, compat_args)
209210

210211

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

0 commit comments

Comments
 (0)