Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@@ -11,7 +11,7 @@ | |||
import pytest | |||
import matplotlib as mpl | |||
from matplotlib import style | |||
from matplotlib importpyplot 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, doingimport 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
ImportanceOfBeingErnest commentedMar 21, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Is the intended consequence of this PR that
is not working any more as inthis example and that instead youhave to use it as a context?
The latter is shown inthis example. Would it be possible to issue a warning if a function is used without a context? |
anntzer commentedMar 21, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 🐑 ) |
Uh oh!
There was an error while loading.Please reload this page.
Fixes#9520, replaces#9521.
Turns out ExitStack was not the easiest solution (as it gets (reasonably) implicitly exited upon GC).
PR Summary
PR Checklist