Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport PR #39451: BUG: raise when sort_index with ascending=None #40083

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/source/whatsnew/v1.2.3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +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).

.. ---------------------------------------------------------------------------

Expand Down
4 changes: 2 additions & 2 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -5482,7 +5482,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",
Expand All @@ -5503,7 +5503,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
Expand Down
5 changes: 4 additions & 1 deletion pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
from pandas.errors import AbstractMethodError, InvalidIndexError
from pandas.util._decorators import doc, rewrite_axis_style_signature
from pandas.util._validators import (
validate_ascending,
validate_bool_kwarg,
validate_fillna_kwargs,
validate_percentile,
Expand Down Expand Up @@ -4518,7 +4519,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",
Expand All @@ -4529,6 +4530,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(
Expand Down
13 changes: 8 additions & 5 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
Iterable,
List,
Optional,
Sequence,
Tuple,
Type,
Union,
cast,
)
import warnings

Expand Down Expand Up @@ -3065,7 +3067,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",
Expand All @@ -3083,7 +3085,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.
Expand Down Expand Up @@ -3243,6 +3245,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"
Expand All @@ -3257,7 +3260,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]
Expand All @@ -3275,7 +3278,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",
Expand All @@ -3295,7 +3298,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
Expand Down
3 changes: 2 additions & 1 deletion pandas/core/sorting.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
Iterable,
List,
Optional,
Sequence,
Tuple,
Union,
)
Expand Down Expand Up @@ -39,7 +40,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,
Expand Down
17 changes: 17 additions & 0 deletions pandas/tests/frame/methods/test_sort_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,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):
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/series/methods/test_sort_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,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):
Expand Down
49 changes: 45 additions & 4 deletions pandas/util/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Module that contains many useful utilities
for validating data or function arguments
"""
from typing import Iterable, Union
from typing import Iterable, Sequence, Union
import warnings

import numpy as np
Expand Down Expand Up @@ -205,9 +205,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__}."
Expand Down Expand Up @@ -381,3 +411,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]