Skip to content

Commit c98d004

Browse files
gaikwadrahul20feefladder
authored andcommitted
Fixed bug: Dataframe.sort_values not raising ValueError for ascending-incompatible value and Series.sort_values raising ValueError for int value (pandas-dev#42684)
1 parent 8fa8657 commit c98d004

File tree

5 files changed

+49
-6
lines changed

5 files changed

+49
-6
lines changed

doc/source/whatsnew/v1.4.0.rst

+2
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ Indexing
229229
- Bug in :meth:`Series.loc` when with a :class:`MultiIndex` whose first level contains only ``np.nan`` values (:issue:`42055`)
230230
- 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`)
231231
- 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`)
232+
- Bug in :meth:`DataFrame.sort_values` and :meth:`Series.sort_values` when passing an ascending value, failed to raise or incorrectly raising ``ValueError`` (:issue:`41634`)
233+
- Bug in updating values of :class:`pandas.Series` using boolean index, created by using :meth:`pandas.DataFrame.pop` (:issue:`42530`)
232234

233235
Missing
234236
^^^^^^^

pandas/core/frame.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
rewrite_axis_style_signature,
8686
)
8787
from pandas.util._validators import (
88+
validate_ascending,
8889
validate_axis_style_args,
8990
validate_bool_kwarg,
9091
validate_percentile,
@@ -6202,7 +6203,7 @@ def sort_values( # type: ignore[override]
62026203
):
62036204
inplace = validate_bool_kwarg(inplace, "inplace")
62046205
axis = self._get_axis_number(axis)
6205-
6206+
ascending = validate_ascending(ascending)
62066207
if not isinstance(by, list):
62076208
by = [by]
62086209
if is_sequence(ascending) and len(by) != len(ascending):

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,
@@ -3438,8 +3438,7 @@ def sort_values(
34383438
)
34393439
ascending = ascending[0]
34403440

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

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

pandas/tests/frame/methods/test_sort_values.py

+20
Original file line numberDiff line numberDiff line change
@@ -868,3 +868,23 @@ def test_sort_values_pos_args_deprecation(self):
868868
result = df.sort_values("a", 0)
869869
expected = DataFrame({"a": [1, 2, 3]})
870870
tm.assert_frame_equal(result, expected)
871+
872+
def test_sort_values_validate_ascending_for_value_error(self):
873+
# GH41634
874+
df = DataFrame({"D": [23, 7, 21]})
875+
876+
msg = 'For argument "ascending" expected type bool, received type str.'
877+
with pytest.raises(ValueError, match=msg):
878+
df.sort_values(by="D", ascending="False")
879+
880+
@pytest.mark.parametrize("ascending", [False, 0, 1, True])
881+
def test_sort_values_validate_ascending_functional(self, ascending):
882+
df = DataFrame({"D": [23, 7, 21]})
883+
indexer = df["D"].argsort().values
884+
885+
if not ascending:
886+
indexer = indexer[::-1]
887+
888+
expected = df.loc[df.index[indexer]]
889+
result = df.sort_values(by="D", ascending=ascending)
890+
tm.assert_frame_equal(result, expected)

pandas/tests/series/methods/test_sort_values.py

+23-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,27 @@ 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_for_value_error(self):
210+
# GH41634
211+
ser = Series([23, 7, 21])
212+
213+
msg = 'For argument "ascending" expected type bool, received type str.'
214+
with pytest.raises(ValueError, match=msg):
215+
ser.sort_values(ascending="False")
216+
217+
@pytest.mark.parametrize("ascending", [False, 0, 1, True])
218+
def test_sort_values_validate_ascending_functional(self, ascending):
219+
# GH41634
220+
ser = Series([23, 7, 21])
221+
expected = np.sort(ser.values)
222+
223+
sorted_ser = ser.sort_values(ascending=ascending)
224+
if not ascending:
225+
expected = expected[::-1]
226+
227+
result = sorted_ser.values
228+
tm.assert_numpy_array_equal(result, expected)
229+
209230

210231
class TestSeriesSortingKey:
211232
def test_sort_values_key(self):

0 commit comments

Comments
 (0)