-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix xkcd() not resetting context anymore. #9603
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
@@ -11,7 +11,7 @@ | |||
import pytest | |||
|
|||
import matplotlib as mpl | |||
from matplotlib import style | |||
from matplotlib import pyplot as plt, style |
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.
Just out of curiosity, is this import style "ok"?
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.
Yeah, doing import sys, os
is what is discouraged.
In that case that you are importing mulitple top level things is obscured, in this case we are importing multiple things from the same namespace (and renaming one). https://www.python.org/dev/peps/pep-0008/#imports
Is the intended consequence of this PR that
is not working any more as in this example and that instead you have to use it as a context?
The latter is shown in this example. Would it be possible to issue a warning if a function is used without a context? |
No, that was not the intent and is a bug. Basically, right now we have a test that the requested pattern works:
but that's only because the cyclic gc (usually) doesn't trigger just before the second assert; as:
always gives None, None. |
Bigger picture: Is the |
I don't have any problems with it sticking around, although switching to pythonic styles (per the "Python-syntax matplotlibrc" PR) would make this infinitely easier (would become a regular style). |
I'd work on pythonic styles first, rather than trying to get this to work again. |
As I see it the Or maybe just because it's great fun. 😎 |
We discussed the pythonic-mplrc proposal during this week's dev call (btw you should join, it's fun :-)) and looks like we'll make some progress on that. |
Glad to hear that things are moving :) (did not read the notes from this week's call yet 🐑 ) |
Fixes #9520, replaces #9521.
Turns out ExitStack was not the easiest solution (as it gets (reasonably) implicitly exited upon GC).
PR Summary
PR Checklist