diff --git a/doc/source/whatsnew/v1.2.3.rst b/doc/source/whatsnew/v1.2.3.rst index 28fc83459b69d..f72ee78bf243a 100644 --- a/doc/source/whatsnew/v1.2.3.rst +++ b/doc/source/whatsnew/v1.2.3.rst @@ -19,7 +19,11 @@ Fixed regressions - Fixed regression in nullable integer unary ops propagating mask on assignment (:issue:`39943`) - Fixed regression in :meth:`DataFrame.__setitem__` not aligning :class:`DataFrame` on right-hand side for boolean indexer (:issue:`39931`) - 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`) -- +- Fixed regression in :meth:`~Series.sort_index` and :meth:`~DataFrame.sort_index`, + which exited with an ungraceful error when having kwarg ``ascending=None`` passed (:issue:`39434`). + Passing ``ascending=None`` is still considered invalid, + and the new error message suggests a proper usage + (``ascending`` must be a boolean or a list-like boolean). .. --------------------------------------------------------------------------- diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 2c95e65c70899..a39aef9b91662 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -5625,7 +5625,7 @@ def sort_index( self, axis: Axis = 0, level: Optional[Level] = None, - ascending: bool = True, + ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True, inplace: bool = False, kind: str = "quicksort", na_position: str = "last", @@ -5646,7 +5646,7 @@ def sort_index( and 1 identifies the columns. level : int or level name or list of ints or list of level names If not None, sort on values in specified index level(s). - ascending : bool or list of bools, default True + ascending : bool or list-like of bools, default True Sort ascending vs. descending. When the index is a MultiIndex the sort direction can be controlled for each level individually. inplace : bool, default False diff --git a/pandas/core/generic.py b/pandas/core/generic.py index eb4c5c07af2c4..4774045849eb6 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -71,6 +71,7 @@ rewrite_axis_style_signature, ) from pandas.util._validators import ( + validate_ascending, validate_bool_kwarg, validate_fillna_kwargs, ) @@ -4548,7 +4549,7 @@ def sort_index( self, axis=0, level=None, - ascending: bool_t = True, + ascending: Union[Union[bool_t, int], Sequence[Union[bool_t, int]]] = True, inplace: bool_t = False, kind: str = "quicksort", na_position: str = "last", @@ -4559,6 +4560,8 @@ def sort_index( inplace = validate_bool_kwarg(inplace, "inplace") axis = self._get_axis_number(axis) + ascending = validate_ascending(ascending) + target = self._get_axis(axis) indexer = get_indexer_indexer( diff --git a/pandas/core/series.py b/pandas/core/series.py index 50a537aeb8623..5fece72ccddca 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -15,9 +15,11 @@ Iterable, List, Optional, + Sequence, Tuple, Type, Union, + cast, ) import warnings @@ -3099,7 +3101,7 @@ def update(self, other) -> None: def sort_values( self, axis=0, - ascending=True, + ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True, inplace: bool = False, kind: str = "quicksort", na_position: str = "last", @@ -3117,7 +3119,7 @@ def sort_values( axis : {0 or 'index'}, default 0 Axis to direct sorting. The value 'index' is accepted for compatibility with DataFrame.sort_values. - ascending : bool, default True + ascending : bool or list of bools, default True If True, sort values in ascending order, otherwise descending. inplace : bool, default False If True, perform operation in-place. @@ -3277,6 +3279,7 @@ def sort_values( ) if is_list_like(ascending): + ascending = cast(Sequence[Union[bool, int]], ascending) if len(ascending) != 1: raise ValueError( f"Length of ascending ({len(ascending)}) must be 1 for Series" @@ -3291,7 +3294,7 @@ def sort_values( # GH 35922. Make sorting stable by leveraging nargsort values_to_sort = ensure_key_mapped(self, key)._values if key else self._values - sorted_index = nargsort(values_to_sort, kind, ascending, na_position) + sorted_index = nargsort(values_to_sort, kind, bool(ascending), na_position) result = self._constructor( self._values[sorted_index], index=self.index[sorted_index] @@ -3309,7 +3312,7 @@ def sort_index( self, axis=0, level=None, - ascending: bool = True, + ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True, inplace: bool = False, kind: str = "quicksort", na_position: str = "last", @@ -3329,7 +3332,7 @@ def sort_index( Axis to direct sorting. This can only be 0 for Series. level : int, optional If not None, sort on values in specified index level(s). - ascending : bool or list of bools, default True + ascending : bool or list-like of bools, default True Sort ascending vs. descending. When the index is a MultiIndex the sort direction can be controlled for each level individually. inplace : bool, default False diff --git a/pandas/core/sorting.py b/pandas/core/sorting.py index 67863036929b3..0195969de1f17 100644 --- a/pandas/core/sorting.py +++ b/pandas/core/sorting.py @@ -10,6 +10,7 @@ Iterable, List, Optional, + Sequence, Tuple, Union, ) @@ -45,7 +46,7 @@ def get_indexer_indexer( target: Index, level: Union[str, int, List[str], List[int]], - ascending: bool, + ascending: Union[Sequence[Union[bool, int]], Union[bool, int]], kind: str, na_position: str, sort_remaining: bool, diff --git a/pandas/tests/frame/methods/test_sort_index.py b/pandas/tests/frame/methods/test_sort_index.py index 221296bfd6d76..5fa60b55f4e21 100644 --- a/pandas/tests/frame/methods/test_sort_index.py +++ b/pandas/tests/frame/methods/test_sort_index.py @@ -761,6 +761,23 @@ def test_sort_index_with_categories(self, categories): ) tm.assert_frame_equal(result, expected) + @pytest.mark.parametrize( + "ascending", + [ + None, + [True, None], + [False, "True"], + ], + ) + def test_sort_index_ascending_bad_value_raises(self, ascending): + # GH 39434 + df = DataFrame(np.arange(64)) + length = len(df.index) + df.index = [(i - length / 2) % length for i in range(length)] + match = 'For argument "ascending" expected type bool' + with pytest.raises(ValueError, match=match): + df.sort_index(axis=0, ascending=ascending, na_position="first") + class TestDataFrameSortIndexKey: def test_sort_multi_index_key(self): diff --git a/pandas/tests/series/methods/test_sort_index.py b/pandas/tests/series/methods/test_sort_index.py index d70abe2311acd..4df6f52e0fff4 100644 --- a/pandas/tests/series/methods/test_sort_index.py +++ b/pandas/tests/series/methods/test_sort_index.py @@ -203,6 +203,20 @@ def test_sort_index_ascending_list(self): expected = ser.iloc[[0, 4, 1, 5, 2, 6, 3, 7]] tm.assert_series_equal(result, expected) + @pytest.mark.parametrize( + "ascending", + [ + None, + (True, None), + (False, "True"), + ], + ) + def test_sort_index_ascending_bad_value_raises(self, ascending): + ser = Series(range(10), index=[0, 3, 2, 1, 4, 5, 7, 6, 8, 9]) + match = 'For argument "ascending" expected type bool' + with pytest.raises(ValueError, match=match): + ser.sort_index(ascending=ascending) + class TestSeriesSortIndexKey: def test_sort_index_multiindex_key(self): diff --git a/pandas/util/_validators.py b/pandas/util/_validators.py index 60a81ed63b005..087dccfadcce1 100644 --- a/pandas/util/_validators.py +++ b/pandas/util/_validators.py @@ -4,6 +4,7 @@ """ from typing import ( Iterable, + Sequence, Union, ) import warnings @@ -208,9 +209,39 @@ def validate_args_and_kwargs(fname, args, kwargs, max_fname_arg_count, compat_ar validate_kwargs(fname, kwargs, compat_args) -def validate_bool_kwarg(value, arg_name): - """ Ensures that argument passed in arg_name is of type bool. """ - if not (is_bool(value) or value is None): +def validate_bool_kwarg(value, arg_name, none_allowed=True, int_allowed=False): + """ + Ensure that argument passed in arg_name can be interpreted as boolean. + + Parameters + ---------- + value : bool + Value to be validated. + arg_name : str + Name of the argument. To be reflected in the error message. + none_allowed : bool, default True + Whether to consider None to be a valid boolean. + int_allowed : bool, default False + Whether to consider integer value to be a valid boolean. + + Returns + ------- + value + The same value as input. + + Raises + ------ + ValueError + If the value is not a valid boolean. + """ + good_value = is_bool(value) + if none_allowed: + good_value = good_value or value is None + + if int_allowed: + good_value = good_value or isinstance(value, int) + + if not good_value: raise ValueError( f'For argument "{arg_name}" expected type bool, received ' f"type {type(value).__name__}." @@ -384,3 +415,14 @@ def validate_percentile(q: Union[float, Iterable[float]]) -> np.ndarray: if not all(0 <= qs <= 1 for qs in q_arr): raise ValueError(msg.format(q_arr / 100.0)) return q_arr + + +def validate_ascending( + ascending: Union[Union[bool, int], Sequence[Union[bool, int]]] = True, +): + """Validate ``ascending`` kwargs for ``sort_index`` method.""" + kwargs = {"none_allowed": False, "int_allowed": True} + if not isinstance(ascending, (list, tuple)): + return validate_bool_kwarg(ascending, "ascending", **kwargs) + + return [validate_bool_kwarg(item, "ascending", **kwargs) for item in ascending]