-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Use DeprecationWarning instead of FutureWarning for is_.._dtype deprecations #55703
Conversation
People may have not upgraded, but downstream packages? If a package is dependent on pandas I would imagine it is likely to become "2.1-compliant" faster than end users. Though hopefully packages are not handling warnings by suppressing them 😆 If they are though, and they are suppressing the FutureWarning, they will have to change it to a DeprecationWarning, then back to FutureWarning (assuming they haven't addressed it). I'm on the fence here. |
Taking the example of seaborn, which is a well maintained package, that only had a release fixing this a few weeks after the pandas release. And I expect there to be many packages that are slower than the seaborn maintainer (note that it is not enough to fix this but also to release). |
And indeed, if they addressed the warning, they either fixed it (and this PR won't change anything for them), or either they suppressed it temporarily, and then they will see the warning again (requiring them to suppress it temporarily again, or prompt them to actually fix it) |
I'm good with changing to a DeprecationWarning, but would like to get thoughts from those who originally deprecated. cc @jbrockmendel @phofl @mroeschke |
fine by me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This failure looks related. https://github.com/pandas-dev/pandas/actions/runs/6653673736/job/18080148037?pr=55703#step:8:107
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Merge when ready @rhshadrach
Thanks @jorisvandenbossche |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
…`FutureWarning` (#14617) This PR changes all FutureWarning's to DeprecationWarning's to match with pandas: pandas-dev/pandas#55703 On pandas_2.0_feature_branch: = 501 failed, 101106 passed, 2071 skipped, 786 xfailed, 312 xpassed, 20 errors in 1234.91s (0:20:34) = This PR: = 445 failed, 101162 passed, 2071 skipped, 786 xfailed, 312 xpassed, 20 errors in 1216.79s (0:20:16) =
xref #54970
Those kind of helpers functions are, I think, for a large part used by other libraries, and to a lesser extent by direct user code. For that kind of deprecations, we first start with a DeprecationWarning, so it's not visible for the end-user (like users of seaborn got a flood of warnings before their latest release).
Of course, changing it now might also give some churn in case some other projects already added some code to suppress them, but I think it's still useful to do now (many people have not yet upgraded to 2.1).