Skip to content

Replace subplot(ijk) calls by subplots(i, j)#18567

Merged
dopplershift merged 1 commit intomatplotlib:masterfrom
timhoffm:subplot-cleanup
Sep 25, 2020
Merged

Replace subplot(ijk) calls by subplots(i, j)#18567
dopplershift merged 1 commit intomatplotlib:masterfrom
timhoffm:subplot-cleanup

Conversation

@timhoffm
Copy link
Copy Markdown
Member

PR Summary

Addresses part of the cases of #17335.

Plus some minor cleanup along the way.

@timhoffm timhoffm added this to the v3.4.0 milestone Sep 24, 2020
dtn = mdates.date2num(dt)

fig = plt.figure()
fig, ((ax1, ax2), (ax3, ax4)) = plt.subplots(2, 2)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of this idiom versus fig, axs = plt.subplots(2, 2). You lose the ability to iterate if you spell them all out like this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think which is better depends on what you are doing (a grid of similar Axes or a grid of very different Axes).

In this case the code was previously using axN so this is the minimal touch to move away from add_subplot(XYZ).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I think it also depends on the names you use,

fig, (ax_hist, ax_map) = plt.subplots(2)

seems much better that slicing into an array (which is similar to the argument of why subplot_mosaic returns a dict).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @tacaswell, especially since this minimizes changes.

dtn = mdates.date2num(dt)

fig = plt.figure()
fig, ((ax1, ax2), (ax3, ax4)) = plt.subplots(2, 2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @tacaswell, especially since this minimizes changes.

@dopplershift dopplershift merged commit eeb32ac into matplotlib:master Sep 25, 2020
@timhoffm timhoffm deleted the subplot-cleanup branch September 25, 2020 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants