Skip to content

Docs issue for pd.concat #41518

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

Closed
softwaredevPK opened this issue May 17, 2021 · 4 comments · Fixed by #44629
Closed

Docs issue for pd.concat #41518

softwaredevPK opened this issue May 17, 2021 · 4 comments · Fixed by #44629
Labels
Deprecate Functionality to remove in pandas Error Reporting Incorrect or improved errors from pandas Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Milestone

Comments

@softwaredevPK
Copy link

softwaredevPK commented May 17, 2021

Hi,

I was recently updating my pandas from 0.24.2 to 1.1.5 and future warning informed me that default value for sort has changed from None to False. It said that to present current behavior I should change it to sort=True. After i updated my package, I checked if sort=None is still accessible and it is, while docs says it accepts only bool.

May I understand that I can still use sort=None to present old behavior? Or should it be depreciated and raise Error?

image

@mzeitlin11
Copy link
Member

Thanks for the report @softwaredevPK! Since the docs and all the type hints along this path specify sort as a bool, I don't think sort=None is meant as a legal input. Based on the code, I'd guess sort=None will happen to be equivalent to sort=False since that gives the same behavior for a check like if sort:

Seems like an easy fix would be to use

def validate_bool_kwarg(value, arg_name, none_allowed=True, int_allowed=False):

to explicitly disallow None as an input

@mzeitlin11 mzeitlin11 added Error Reporting Incorrect or improved errors from pandas Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels May 18, 2021
@mzeitlin11 mzeitlin11 added this to the Contributions Welcome milestone May 18, 2021
@phofl
Copy link
Member

phofl commented May 18, 2021

@mzeitlin11 would you be ok with raising a FutureWarning in 1.3 to remove the None behavior in 2.0? The Message kinda suggests that None may be a valid input (which doesn‘t really makes sense of course, but I don‘t know how people use this)

@softwaredevPK
Copy link
Author

softwaredevPK commented May 18, 2021

With colleague of mine we performed few tests to check if there is any difference between sort=None vs sort=False vs sort=True.
As You can expect sort=None vs sort=True has completely different behavior but when you compare None with False it become interesting.
Test was made on few simple DF. When we were using sort=False each result was not sorted. Fort sort=None we found that it was sorting result when indexes weren't same.

So in my opinion none shouldn't be allowed ;-)

@mzeitlin11
Copy link
Member

@mzeitlin11 would you be ok with raising a FutureWarning in 1.3 to remove the None behavior in 2.0?

Sounds good to me!

@lithomas1 lithomas1 added the Deprecate Functionality to remove in pandas label May 19, 2021
@mroeschke mroeschke modified the milestones: Contributions Welcome, 1.4 Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Error Reporting Incorrect or improved errors from pandas Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants