-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Delay warning for deprecated parameter 'vert' of box and violin #29155
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
Conversation
I would suggest targeting |
@ksunden do you have a link to the issue? This is a fundamental problem with deprecations. When you introduce a deprecation third-party libraries will see it. This will have two effects:
I believe that you cannot reasonably circumvent version gating in third party libraries. Even if we give one release of transition where both versions are available equally, they have to pin matplotlib quite aggressively. It's rather the second point that motivates me for pending releases. By default, we currently do not do pending deprecations. Is that something we should should regularly do and add to our API change guideline? OTOH I'm not aware of substantial complaints on our current deprecation strategy. |
Assuming pandas is following SPEC0, does this mean we should wait until May 2026 (drop date for mpl3.9 support) to deprecate the old parameter? |
This was discovered by the run of Fedora builds that @QuLogic did, rather than being specifically reported/having an issue to link to. I discussed it with @tacaswell and we came to the decision to delay it by just one release, which still may incur version gates on libraries, but minimizes impact to especially downstream applications (which tend to have narrower support windows). If the new API were available previously, I would be less worried about it, even if a version without fell within Pandas' minimal support window, since the further downstream effects are minimized. |
41ee6b2
to
1564012
Compare
As for release notes, I will take that on in release, not including them in the compiled form and leaving them in next... But thanks for the reminder there. |
I would like to give one version with both the old and new way not warning before we warn on the old way to reduce the impact of the transition. What got me with this one is that use are something like:
where pandas is just passing |
The new parameter, 'orientation', was only added in the current release, so downstream wanting to avoid warnings would require version gates. Therefore delaying by at least one release to ease the transition. This was motivated by Pandas failing tests on this warning.
1564012
to
f5ce789
Compare
The new parameter, 'orientation', was only added in the current release, so downstream wanting to avoid warnings would require version gates. Therefore delaying by at least one release to ease the transition. This was motivated by Pandas failing tests on this warning. This portion is made direct to v3.10.x with matplotlib#29155 being the accompanying version to edit the deprecation on main.
The tests need updating to match the changes here. |
Coming back to the general question: Do we need to refine our deprecation policy? e.g.
|
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.
These are not pending anymore in 3.11
I'm overall not clear what we want. The comments and code here and in #29159 send mixed signals a. 3.10: discourage, 3.11: deprecate I would have expected c. |
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
I was largely thinking a) discourage in 3.10, deprecate in 3.11. A pending deprecation still does fail Pandas tests, so if we find "don't break pandas with our release" motivating, that doesn't solve that issue. |
As far as I understand #29155 (comment). The failure of pandas test should come from the way the pandas test code is written not from the pandas code itself. That's an important difference because then users of pandas don't rave to wait for a pandas release. In other words, pandas can right now change their tests to fix this - yes this requires version gating to get a smooth transition, but pandas has a Minimum Version CI run (currently using Matplotlib 3.6) and unless they aggressively pin to 3.10 in the future they'll have to do version gating anyway. On plus side, other libraries would become aware through the pending deprecation. |
That is also my understanding and I agree that pandas putting version gating in their tests is not a big deal.
However, if we add the new way and deprecate the old way in one step we are going to inflict version gating on user's code which I think is a problem. |
That's why
That's why I suggested a pending deprecation. Users should not see these. |
Ok, then I think we agree! I get lost in the details of what warnings users do/do not see under what situations. |
@ksunden will do a independent PR to 3.10.x and handle the (likely) conflicts on the merge-up. |
The new parameter, 'orientation', was only added in the current release, so downstream wanting to avoid warnings would require version gates. Therefore delaying by at least one release to ease the transition. This was motivated by Pandas failing tests on this warning. This portion is made direct to v3.10.x with matplotlib#29155 being the accompanying version to edit the deprecation on main. Reinstate warning as a pending warning STY: fits on one line
The new parameter, 'orientation', was only added in the current release, so downstream wanting to avoid warnings
would require version gates. Therefore delaying by at least one release to ease the transition.
This was motivated by Pandas failing tests on this warning.
PR summary
PR checklist