convert some sample plots to use plt.subplots() instead of other methods #17340

asafmaman101 · 2020-05-06T11:12:16Z

PR Summary

Closes #17023. I modified the following sample plots, as issued:

"Multiple Subplots" (subplot),
"streamplot" (fig.add_subplots),
"Polar Plots" (subplot),
XKCD (fig.add_axes).

EXCLUDING

"Surface3d" (gca)

Since, that one was fixed in PR #16559

PR Checklist

Has Pytest style unit tests
Code is Flake 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

QuLogic · 2020-05-06T20:20:28Z

examples/images_contours_and_fields/plot_streamplot.py

 ax0.streamplot(X, Y, U, V, density=[0.5, 1])
 ax0.set_title('Varying Density')

 # Varying color along a streamline
-ax1 = fig.add_subplot(gs[0, 1])
+plt.sca(ax1)


I don't see why this is needed.

Hi, thanks for the review. I deleted line 34 to fix the issue (of consistant use of .subplots() instead of different method in each sample) and in line 35 I added sca() in order to display the colorbar next to the right axes.

I think it would be better to pass the axes to fig.colorbar (as fig.calorbar(strm.lines, ax=axN)) instead. That will explicitly tell colorbar which axes to steal the space from rather than relying on the global state.

QuLogic · 2020-05-06T20:25:09Z

examples/images_contours_and_fields/plot_streamplot.py

@@ -61,6 +61,9 @@
 U[:20, :20] = np.nan
 U = np.ma.array(U, mask=mask)

+gs = gridspec.GridSpec(nrows=3, ncols=2, height_ratios=[1, 1, 2])


Not sure this is the best way to do this. @jklymak?

Yeah, don't create a new gridspec. You can get the old one with gs = axs.get_subpltospec().get_gridspec().

QuLogic · 2020-05-06T20:26:33Z

examples/subplots_axes_and_figures/subplot.py

@@ -15,14 +15,14 @@
 y1 = np.cos(2 * np.pi * x1) * np.exp(-x1)
 y2 = np.cos(2 * np.pi * x2)

-plt.subplot(2, 1, 1)


From the filename, I'm not sure if this file is actually about subplot...

Did you mean it should be an example of the subplot() function? The page is referenced from the section “ Multiple Subplots”, so I guess it does not specifically refer to subplot().

I think its reasonable to have what we feel is the canonical way here.

Can I suggest you add a second figure that shows subplot though with a bit of preceding text that says it is an alternate way using pyplot?

BTW, the rest seems fine. Ping for a review, and thanks for working on this!

Not sure I fully understand what you mean, you recommand adding a second example in the same page with the original method of subplot?

Yes. Each example can have multiple figures on it.

tacaswell · 2020-05-07T04:11:53Z

Thanks for working on this @asafmaman101 !

jklymak

So I don't agree with the use of subplots for the stream plot example. Adding axes and then erasing them is confusing. If you are going to have multi-row or column axes based on gridspec, the original code is the best way to do it, in my opinion.

jklymak · 2020-05-07T04:29:56Z

examples/images_contours_and_fields/plot_streamplot.py

-gs = gridspec.GridSpec(nrows=3, ncols=2, height_ratios=[1, 1, 2])
+
+gs = {'height_ratios': [1, 1, 2]}
+fig, axes = plt.subplots(nrows=3, ncols=2, gridspec_kw=gs, figsize=(7, 9))


We usually use axs for a list of axes.

jklymak · 2020-05-07T04:30:21Z

examples/images_contours_and_fields/plot_streamplot.py

+
+gs = {'height_ratios': [1, 1, 2]}
+fig, axes = plt.subplots(nrows=3, ncols=2, gridspec_kw=gs, figsize=(7, 9))
+((ax0, ax1), (ax2, ax3), (ax4, ax5)) = axes


There is really no need to unpack these, so don't do so. Just refer to axs[0, 1] etc.

jklymak · 2020-05-07T04:32:22Z

examples/images_contours_and_fields/plot_streamplot.py

@@ -61,6 +61,9 @@
 U[:20, :20] = np.nan
 U = np.ma.array(U, mask=mask)

+gs = gridspec.GridSpec(nrows=3, ncols=2, height_ratios=[1, 1, 2])


Yeah, don't create a new gridspec. You can get the old one with gs = axs.get_subpltospec().get_gridspec().

…ng and changing varibale name

asafmaman101 · 2020-05-09T07:10:02Z

Hi, I modified the files as discussed above. How does it look now? Thanks.

jklymak · 2020-05-09T20:45:29Z

examples/images_contours_and_fields/plot_streamplot.py


 # Create a mask
 mask = np.zeros(U.shape, dtype=bool)
 mask[40:60, 40:60] = True
 U[:20, :20] = np.nan
 U = np.ma.array(U, mask=mask)

+gs = axs[0, 0].get_subplotspec().get_gridspec()
+axs[2, 0].remove()
+axs[2, 1].remove()


Again, I don't think we should use example with ax.remove() in them. I'd prefer we just added each of these (i.e. the old behaviour was fine). But maybe other folks have other opinions.

Hi,
I agree it was a bit messy to use the subplots method here. I reverted the file to the original version, as you recommended.
How are the other files?
Also, I cannot figure out why the codecov test does not pass sometimes.
Thanks

timhoffm

Thanks for working on this. And sorry we're requesting to revert parts. We should have been better curating #17023 for desired changes.

timhoffm · 2020-05-17T21:25:30Z

examples/showcase/xkcd.py

-    fig = plt.figure()
-    ax = fig.add_axes((0.1, 0.2, 0.8, 0.7))
+    fig, ax = plt.subplots()
+    ax.set_position((0.1, 0.2, 0.8, 0.7))


add_axes() is specifically designed for adding axes at arbitrary locations. IMHO it's the right method to use here. Creating a figure and then creating an axes at a specific position is conceptually simpler than creating a figure and an axes in the default layout and then moving the existing axes raound.

subplots sets up a gridspec for the underlying axes, whereas if you are just going to move it that gridspec is not needed. So there is some advantage to add_axes here.

Hi, no problem I can revert it to the original version, but I want to make sure I understand - you suggest to keep it the same way it was in the beginning? or some other method using both subplots and add_axes (if it's feasible, didn't already thought about it to details).

That example was already correct as it was originally. Please revert your change.

timhoffm

The commented issues cause the sphinx build failure.

examples/subplots_axes_and_figures/subplot.py

asafmaman101 added 2 commits May 6, 2020 13:55

convert some sample plots to use plt.subplots() instead of other methods

a2765d5

fixing line length

3c4faca

asafmaman101 mentioned this pull request May 6, 2020

DOC: Tutorial/Sample plots should use same fig/axis creation method #17023

Closed

QuLogic reviewed May 6, 2020

View reviewed changes

tacaswell added this to the v3.4.0 milestone May 7, 2020

jklymak reviewed May 7, 2020

View reviewed changes

asafmaman101 added 2 commits May 7, 2020 09:19

changing the use of gridspec() to avoid repeatation, removing unpacki…

cd25cea

…ng and changing varibale name

fix exeeded line while linting test

2ae3dd9

jklymak reviewed May 9, 2020

View reviewed changes

revert plot_streamplot.py to original version

1e58203

asafmaman101 requested a review from jklymak May 11, 2020 13:43

jklymak added the Documentation label May 11, 2020

asafmaman101 added 2 commits May 17, 2020 09:46

adding both methods how to generate multiple plots

a664c8a

fix linting

a9d580e

timhoffm reviewed May 17, 2020

View reviewed changes

revert xkcd.py

bc444fa

asafmaman101 requested review from timhoffm and tacaswell May 19, 20 10A8F 20 13:50

timhoffm reviewed May 24, 2020

View reviewed changes

examples/subplots_axes_and_figures/subplot.py Outdated Show resolved Hide resolved

examples/subplots_axes_and_figures/subplot.py Outdated Show resolved Hide resolved

modify heading in subplot.py

820bbdf

timhoffm approved these changes May 25, 2020

View reviewed changes

timhoffm modified the milestones: v3.4.0, v3.3.0 May 25, 2020

timhoffm merged commit 36bc7da into matplotlib:master May 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

convert some sample plots to use plt.subplots() instead of other methods #17340

convert some sample plots to use plt.subplots() instead of other methods #17340

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

convert some sample plots to use plt.subplots() instead of other methods #17340

convert some sample plots to use plt.subplots() instead of other methods #17340

Uh oh!

Conversation

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!