Skip to content

MatplotlibDeprecationWarning removal hard-breaks seaborn in 3.6rc1 #23716

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.

Sign up for GitHub

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

Jump to bottom
Closed
tacaswell opened this issue Aug 23, 2022 · 5 comments · Fixed by #23722
Closed

MatplotlibDeprecationWarning removal hard-breaks seaborn in 3.6rc1 #23716

tacaswell opened this issue Aug 23, 2022 · 5 comments · Fixed by #23722
Assignees
@QuLogic
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone
v3.6.0

Comments

@tacaswell
Copy link
Member

This removal hard breaks a seaborn function.

Also it's rather unlikely for end users to import MatplotlibDeprecationWarning.

Curious what the basis for this claim was? https://github.com/search?q=%22from+matplotlib.cbook+import+MatplotlibDeprecationWarning%22&type=Code shows 2k+ direct imports of this class, and searching for alternate usage patterns turns up others as well.

I don't think we have or easily could have added a warning on importing the class

I agree that this would have been hard! But IMO that puts the burden on matplotlib to deprecate with a bit more care (e.g., proactively searching for usage in downstream libraries, etc.). You are right that this was "properly documented", but it is mentioned in the middle of a very long list of API changes (there are >4000 words on https://matplotlib.org/stable/api/prev_api_changes/api_changes_3.4.0.html). Downstream libraries have the reasonable expectation that they can rely on unit test coverage to surface deprecation warnings, since matplotlib is usually good about that sort of thing.

In the future, please boost the signal of any silent deprecations.

Originally posted by @mwaskom in #22514 (comment)
@tacaswell tacaswell added this to the v3.6.0 milestone Aug 23, 2022
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Aug 23, 2022
@tacaswell
Copy link
Member Author

We should put the alias back with a warning for 3.6.

@mwaskom
Copy link

mwaskom commented Aug 23, 2022

TIL that you can write custom __getattr__ logic at the module-level as of Python 3.7: https://peps.python.org/pep-0562, which I believe is the solution for this sort of problem.

@tacaswell
Copy link
Member Author

Yeah, we have a helper using module level __getattr__ specifically for deprecating module level attributes.

@oscargus
Copy link
Member

I guess just importing MatplotlibDeprecationWarning in cbook/__init__.py would be OK. The problem is to guarantee a warning...

I think that there are other classes which may be in the same situation.

@QuLogic QuLogic self-assigned this Aug 23, 2022
QuLogic added a commit to QuLogic/matplotlib that referenced this issue Aug 24, 2022
@QuLogic
Restore deprecation class aliases in cbook
75a5d7c 
Fixes matplotlib#23716
@QuLogic
Copy link
Member

QuLogic commented Aug 24, 2022

I think that there are other classes which may be in the same situation.

Classes are fine; they can warn with the deprecation decorator. The problem with MatplotlibDeprecationWarning is that it is never instantiated by the user, but only used in except clauses, so it would not trigger any deprecation. The module-level __getattr__ allows warning on access instead.

@QuLogic QuLogic mentioned this issue Aug 24, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@QuLogic QuLogic

Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.6.0
Development

Successfully merging a pull request may close this issue.

Restore deprecation class aliases in cbook QuLogic/matplotlib
4 participants
@tacaswell @QuLogic @mwaskom @oscargus