ENH: Raise error on ascending='False'

[ezerkar]

Is your feature request related to a problem?

when sort_values ascending parameter is set to a string (df.sort_values(col_name, ascending='False') it does not error although it's meaningless, this could be quite confusing.

Describe the solution you'd like

Raise an error if ascending is equal to values that are not boolean

API breaking implications

Error raising

Describe alternatives you've considered

Warning is also good, but I think error makes more sense

Additional context

for an hour I was wondering why ascending='False'
is not working

isinstance('False', bool) :)

[vishalkumarprasad]

Hello, I am a new to open source contribution. Can I take up this enhancement?

[ezerkar]

You can't be newer than me :) If it's up to me, you have my blessings ! Ezer

…

On Mon, May 24, 2021 at 1:15 PM Vishal Kumar Prasad < ***@***.***> wrote: Hello, I am a new to open source contribution. Can I take up this enhancement? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#41634 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPNI5NADVRFMFGDFOLOSMDTPIRL7ANCNFSM45L7FWRQ> .

[lithomas1]

Hi @ezerkar,
This seems reasonable to me, since we do this for sort_index as well. (see #39451)
@vishalkumarprasad Thanks for volunteering to contribute to pandas. Feel free to take up this issue. You can find our contributing guidelines here. https://pandas.pydata.org/docs/development/contributing.html and if you need help, don't hesitate to ping me :).

[vishalkumarprasad]

take

[ezerkar]

Thanks to you all בתאריך יום ב׳, 24 במאי 2021, 20:07, מאת Vishal Kumar Prasad ‏< ***@***.***>:

…

take — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#41634 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPNI5K7K57KOUPOT2QXDNLTPKBW7ANCNFSM45L7FWRQ> .

[Lokeshrathi]

You can use sorted() function here.
Code:

a = ("h", "b", "a", "c", "f", "d", "e", "g")
x = sorted(a)
print(x)

Output:
['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h']

[vishalkumarprasad]

Hi Eric, I am already working on this enhancement.

[Anupam-USP]

Hey, is this issue still open or taken?

[ezerkar]

AFAIK it's taken בתאריך יום ה׳, 27 במאי 2021, 18:07, מאת DaPy15 ***@***.***>:

…

Hey, is this issue still open or taken? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#41634 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPNI5K2T25A4H73SM7PXDDTPZN4DANCNFSM45L7FWRQ> .

[vishalkumarprasad]

Hi all, while working on this problem, i figured another issue where if you put series.sort_values ascending parameter to 1 or 0, it fails. However according to the code, it shouldn't fail.
Can someone confirm?

[ninasiam]

Hello all! @vishalkumarprasad I have tried the test case you have described and it indeed fails. However, I have noticed that the is_bool() function - in pandas,core-, which raises the error, also fails with 0, 1 as input arguments. It seems to me that series.sort_values() works as expected.

[vishalkumarprasad]

Hi all, @ninasiam If you look into the function definition of series.sort_values(), its clearly written that the ascending argument takes either bool or int or a sequence of bool or int. Hence there is mismatch somewhere, either in the function definition or is_bool() function or the implementation of is_bool() function. Please let me know your thoughts.

[ninasiam]

Hello! @vishalkumarprasad I see your point, I have probably missed that :) I think the mismatch is probably due to the way is_bool() function is implemented. If I try:

pd.core.dtypes.common.is_bool(True)
pd.core.dtypes.common.is_bool(1)  

The output is:

True
False

respectively. An easy fix, to avoid confusion for the time being, could be the update of the sort_values() function definition, to accept only True/False as input for the ascending parameter.

[vishalkumarprasad]

Hello @ninasiam,
So there is one more point which actually led me towards this issue. sort_values for series does not work currently for 1 and 0, but sort_values for dataframe works perfectly with 1 and 0. Shouldn't we make both of them to work in the same way?

[gaikwadrahul20]

take