Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
use plt.subplots() in examples as much as possible#1462
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
At the recent LBL Software Carpentry Workshop, it was pointed out that there'san inconsistency within our documentation for how to create new figures withsubplots.Indeed, most examples were using the old way, something like: fig = plt.figure() ax = plt.subplot(111) # or plt.add_subplot(111)This patch changes a whole bunch of instances like the above to: fig, ax = plt.subplots()We should strive to have a minimal amount of constants in our code,especially unusual ones like `111`, which only make sense to Matlabrefugees.I have left unchanged examples which were using axes keywords passed tosubplot() and add_subplot(), since those end up transforming things like: figure() subplot(111, axisbg='w')to plt.subplots(subplot_kw=dict(axisbg='w'))which isn't necessarily better.I also did not touch most of the user_interfaces examples, since those did notinvolve using plt, but instead explicitly imported Figure, and used the OOapproach on Figure instances.Also updated instaces where the old "import pylab as p" convention was used touse our standard "import matplotlib.pyplot as plt"I have also updated some, but not all uses of subplot(121) etc, but I'm a bitexhausted after doing all of these.
| @@ -24,7 +24,7 @@ class BlitQT(QObject): | |||
| def __init__(self): | |||
| QObject.__init__(self, None, "app") | |||
| self.ax =p.subplot(111) | |||
| fig,self.ax =plt.subplots() | |||
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.
I'm not sure whether:
fig, ax = plt.subplots()is better than
ax = plt.subplot(111)or evenax = plt.axes()
Note:
I do agree that:
fig, self.ax = plt.subplots()is better than
fig = plt.figure()ax = plt.subplot(111)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.
this inspired me to make#1475 to have plt.subplot() work just like plt.axes().
I'm happy to change this to just be plt.axes()
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.
Cool. Glad it inspired you! 😄
I think we can use your new syntax here now. Thoughts?
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.
On Tue, Nov 13, 2012 at 1:37 AM, Phil Elsonnotifications@github.comwrote:
Cool. Glad it inspired you! [image: 😄]
I think we can use your new syntax here now. Thoughts?
This is going into 1.2.x and the new feature was merged into master, so we
could only change this once it's merge in master.
This is the sort of bifurcation that I was talking about in the recent
thread on mpl-devel - but no solution is perfect...
I think this is fine to go on the 1.2.x branch, but as the 1.2.0 release is pending today, let's hold off on merging this until after 1.2.0 is tagged. |
The only problem with that is, if we miss a complete clanger in v1.2.0 we will want to release a v1.2.1 pretty quickly, and that will end up being a bigger change than we would ideally like. Just wanted to make that issue clear - I still agree with your suggestion though@mdboom . |
ok, should be all good to go now, I went and flaked out some more unused modules, and found a couple of places where I had missed the fact that |
Personally, I feel this is a rather big change. I also feel this isn't really a bugfix and, as a result, should not be merged into v1.2.x. I understand others feel differently, so I'm happy to back down of the consensus is to merge into the maintenance branch. |
I agree with@dmcdougall. This is much better positioned to go into master, not maintenance. I hadn't realized at first that this was set to go into v1.2.x. |
I'm fine either way, I originally submitted this against The argument for having it go into v1.2.x is that there are no functional changes here - the only change that's not in one of the examples is a simple addition of the description of If this doesn't make it into v1.2.x, should there be 1.2.x releases, all it would mean is that our docs will stay "uglier" for longer. I can live with that, but that's essentially what's at stake here. |
just pinging everyone for a second opinion: as I understand it,@mdboom,@pelson and I are all ok with these documentation-only changes going into I consider this to be fix for a bug in our docs, and would prefer user-facing docs to get this facelift sooner, rather than later. |
Sorry for going incognito everyone. I just started a new job in a different country and I've been snowed under with admin and work. I'd also rather see a v1.3, but I guess the final call goes to@mdboom. Also, don't let my opinion hold this up. If the consensus has one opinion and I have another, I am happy to agree. |
Yeah -- I think I'm coming down on the side of this being better on master after all. This is changing a lot of code after all, even if it is only example code. I think we should be conservative here. |
This is a docs-only update - but I think it'll be worthwhile to start to give a more sensible way of creating figures and axes in one call for our the folks who'll be looking at matplotlib with fresh eyes after the 1.2 release.
But I totally understand if folks feel this is too big of a pill to swallow this late in the game.
At the recent LBL Software Carpentry Workshop, it was pointed out that there's
an inconsistency within our documentation for how to create new figures with
subplots.
Indeed, most examples were using the old way, something like:
This patch changes a whole bunch of instances like the above to:
We should strive to have a minimal amount of constants in our code,
especially unusual ones like
111, which only make sense to Matlabrefugees.
I have left unchanged examples which were using axes keywords passed to
subplot() and add_subplot(), since those end up transforming things like:
to
which isn't necessarily better.
I also did not touch most of the user_interfaces examples, since those did not
involve using plt, but instead explicitly imported Figure, and used the OO
approach on Figure instances.
Also updated instaces where the old "import pylab as p" convention was used to
use our standard "import matplotlib.pyplot as plt"
I have also updated some, but not all uses of subplot(121) etc, but I'm a bit
exhausted after doing all of these.