Skip to content

Deprecation warning #20046

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

Merged
merged 1 commit into from
May 10, 2021
Merged

Conversation

brunobeltran
Copy link
Contributor

@brunobeltran brunobeltran commented Apr 22, 2021

PR Summary

We are currently unable to unintrusively deprecate class-level attributes (because we subclass UserWarning and not DeprecationWarning). Based on a previous dev call, we decided the solution was to "do it anyway". This led to a couple of issues, e.g. #19080 and #19839 and #19850.

The history of the decision to not subclass DeprecationWarning has to do with a decision from core Python in the 2.x days to not show DeprecationWarnings to users. However, there is now a more sophisticated filter in place (see https://www.python.org/dev/peps/pep-0565/).

Users that want to see MatplotlibDeprecationWarning in their CI will now have to export PYTHONWARNING=d, launch python with -Wa, or use warning filters to enable DeprecationWarnings (or MatplotlibDeprecationWarning specifically, depending on their needs).

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@brunobeltran brunobeltran marked this pull request as draft April 22, 2021 19:46
@QuLogic QuLogic added the status: needs comment/discussion needs consensus on next step label Apr 22, 2021
@brunobeltran brunobeltran marked this pull request as ready for review April 22, 2021 21:05
@brunobeltran
Copy link
Contributor Author

CI failures seem unrelated.

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading at @anntzer link I still think this is the right deprecation here

@timhoffm
Copy link
Member

timhoffm commented Apr 28, 2021

Behavior for showing DeprecationWarnings according to https://www.python.org/dev/peps/pep-0565/:

Warning shows, when using deprecated functions

  • in interactive prompts, including notebooks
  • in scripts
  • in pytest runs

Warnings will continue to be silenced by default for packaged code distributed as part of an importable module.

I failed to construct such a case 👀

This still warns:

myscript.py  -- calls -->  mylib.plot() -- calls --> a deprecated Matplotlib function

where mylib is next to myscript. Maybe this is only suppressed if mylib is in site-packages?

Anyway given the above warning cases, I think it's safe enough to switch to DeprecationWarning.

@timhoffm timhoffm added this to the v3.5.0 milestone Apr 28, 2021
@timhoffm
Copy link
Member

Does this need an API change note?

@jklymak
Copy link
Member

jklymak commented May 3, 2021

Definitely needs an API note!

@jklymak jklymak marked this pull request as draft May 3, 2021 14:31
@brunobeltran brunobeltran force-pushed the deprecation-warning branch from ed3d631 to 8c9af69 Compare May 6, 2021 18:07
@brunobeltran brunobeltran marked this pull request as ready for review May 6, 2021 18:12
@brunobeltran
Copy link
Contributor Author

Squashed and added an API note.

Unsure where to dump all the information for most readability? Maybe a blog post on best practices for dealing with warnings for users/CI/library authors and would be more effective than putting some of this information in the docs?

@brunobeltran brunobeltran added status: needs merge and removed status: needs comment/discussion needs consensus on next step labels May 7, 2021
@jklymak jklymak merged commit 674d670 into matplotlib:master May 10, 2021
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Aug 7, 2021
…cy and fix check()

What a mess in tests LOL!

Related tickets:

[1] computationalmodelling/nbval#162
[2] computationalmodelling/nbval#167
[3] ipython/ipython#12817
[4] ipython/ipython#12889
[5] ipython/ipykernel#591
[6] matplotlib/matplotlib#20046

git-svn-id: file:///srv/repos/svn-community/svn@994636 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Aug 7, 2021
…cy and fix check()

What a mess in tests LOL!

Related tickets:

[1] computationalmodelling/nbval#162
[2] computationalmodelling/nbval#167
[3] ipython/ipython#12817
[4] ipython/ipython#12889
[5] ipython/ipykernel#591
[6] matplotlib/matplotlib#20046


git-svn-id: file:///srv/repos/svn-community/svn@994636 9fca08f4-af9d-4005-b8df-a31f2cc04f65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants