Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
Kojoley merged 1 commit intomatplotlib:masterfromanntzer:xkcd
Oct 28, 2017

Conversation

anntzer
Copy link
Contributor

@anntzeranntzer commentedOct 28, 2017
edited by Kojoley
Loading

Fixes#9520, replaces#9521.
Turns out ExitStack was not the easiest solution (as it gets (reasonably) implicitly exited upon GC).

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@@ -11,7 +11,7 @@
import pytest

import matplotlib as mpl
from matplotlib import style
from matplotlib importpyplot as plt,style
Copy link
Member

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"?

Copy link
Member

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

@jklymakjklymak mentioned this pull requestOct 28, 2017
1 task
@KojoleyKojoley merged commit8989a6f intomatplotlib:masterOct 28, 2017
@anntzeranntzer deleted the xkcd branchOctober 28, 2017 21:30
@ImportanceOfBeingErnest
Copy link
Member

ImportanceOfBeingErnest commentedMar 21, 2018
edited
Loading

Is the intended consequence of this PR that

plt.xkcd()plt.figure()plt.plot(...)

is not working any more as inthis example and that instead youhave to use it as a context?

with plt.xkcd():    plt.figure()    plt.plot(...)

The latter is shown inthis example.

Would it be possible to issue a warning if a function is used without a context?

@anntzer
Copy link
ContributorAuthor

anntzer commentedMar 21, 2018
edited
Loading

No, that was not the intent and is a bug.
Sigh, circular references and garbage collection...

Basically, right now we have a test that the requested pattern works:

def test_xkcd_no_cm():    assert mpl.rcParams["path.sketch"] is None    plt.xkcd()    assert mpl.rcParams["path.sketch"] == (1, 100, 2)

but that's only because the cyclic gc (usually) doesn't trigger just before the second assert; as:

from pylab import *import gcprint(rcParams["path.sketch"])plt.xkcd()gc.collect()print(rcParams["path.sketch"])plt.show()

always gives None, None.

@phobson
Copy link
Member

Bigger picture: Is thexkcd style/function/context manager really something that we want to continue supporting?

@anntzer
Copy link
ContributorAuthor

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).

@jklymak
Copy link
Member

I'd work on pythonic styles first, rather than trying to get this to work again.

@anntzeranntzer mentioned this pull requestMar 21, 2018
6 tasks
@ImportanceOfBeingErnest
Copy link
Member

As I see it thexkcd function shows the power of the library. It's kind of an advertisement of what is possible with matplotlib. So yes, I'm totally in favour of keeping that (although I don't care if it's a pyplot function or a style or usable only with contexts etc.).

Or maybe just because it's great fun. 😎

image

afvincent and MSeifert04 reacted with laugh emoji

@afvincent
Copy link
Contributor

@anntzer Well, as you know (#6157), there is a challenge about convertingplt.xkcd into a “regular” style by mean of an rcparams-file because it uses apatheffect rcparams, which cannot be parsed at the moment :/.

@anntzer
Copy link
ContributorAuthor

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.

@afvincent
Copy link
Contributor

Glad to hear that things are moving :) (did not read the notes from this week's call yet 🐑 )

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell approved these changes

@jklymakjklymakjklymak left review comments

@dopplershiftdopplershiftdopplershift approved these changes

@KojoleyKojoleyKojoley approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.1.1
Development

Successfully merging this pull request may close these issues.

XKCD context manager not resetting anymore in 2.1
8 participants
@anntzer@ImportanceOfBeingErnest@phobson@jklymak@afvincent@tacaswell@dopplershift@Kojoley

[8]ページ先頭

©2009-2025 Movatter.jp