Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: make subplot reuse gridspecs if they fit #11441

Closed

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jun 15, 2018

PR Summary

Right now, if we call

import matplotlib.pyplot as plt
fig = plt.figure()

ax = fig.add_subplot(3, 4, 1)
ax2 = fig.add_subplot(3, 2, (3, 5))
print(ax.get_subplotspec().get_gridspec())
print(ax2.get_subplotspec().get_gridspec())

plt.show()

The gridspecs will be different. Thats unfortunate, because there is no reason for these two subplots to not share the same gridspec, and if they do, then constrained_layout will work with them. Note this change will mean that many calls to pyplot.subplot and pyplot.subplot2grid will now work with constrained_layout=True as well.

This PR changes that so they two subplots use the same gridspec.

Note The more fine-grained subplot call must come first, or the grid specs will not "fit", so this is pretty simple minded. However, the PR doesn't make things worse, it just makes two gridspecs like it did before. i.e. if we switch the order of the calls we get two gridspecs:

import matplotlib.pyplot as plt
fig = plt.figure()

ax2 = fig.add_subplot(3, 2, (3, 5))
ax = fig.add_subplot(3, 4, 1)
print(ax.get_subplotspec().get_gridspec())
print(ax2.get_subplotspec().get_gridspec())

plt.show()

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

@jklymak jklymak added topic: geometry manager LayoutEngine, Constrained layout, Tight layout status: work in progress labels Jun 15, 2018
@jklymak jklymak added this to the v3.0 milestone Jun 15, 2018
@jklymak jklymak force-pushed the enh-reuse-gridspec-if-possible branch from 866a194 to 6ccc62a Compare June 15, 2018 18:36
@jklymak jklymak force-pushed the enh-reuse-gridspec-if-possible branch from 6ccc62a to f15a7c7 Compare June 15, 2018 18:52
@jklymak
Copy link
Member Author

jklymak commented Jun 15, 2018

I suppose this is a breaking change if someone was checking the results of ax.get_geometry() in their code and expected the geometry to stay the same:

ax1 = fig.add_subplot(1, 2, 1)
ax2 = fig.add_subplot(1, 1, 1)
print(ax2.get_geometry())

used to return (1, 1, 1), but now returns (1, 2, 1). Note thats get_geometry is pretty crappy function anyways, as it does not return the full geometry specification for an axes; i.e. the actual geometry of ax2 is (1, 2, (1, 2))

@jklymak jklymak modified the milestones: v3.0, v3.1 Jun 21, 2018
@jklymak jklymak modified the milestones: v3.1.0, v3.2.0 Feb 4, 2019
@anntzer
Copy link
Contributor

anntzer commented Mar 3, 2019

Perhaps a better idea would be to just always create a new gridspec with shape (lcm(gs1.nrows, gs2.nrows), lcm(gs1.ncols, gs2.ncols)) and reparent all axes to this gridspec? This would avoid the order dependency.

@jklymak
Copy link
Member Author

jklymak commented Mar 4, 2019

Yes, that could work. Wasn't 100% sure this whole PR was a good idea, rather than just telling people to not use add_subplot, but happy to come back to it...

@anntzer
Copy link
Contributor

anntzer commented Mar 4, 2019

I'm not really sure it's worth it either (well I don't think the implementation would be that complex), just pointing out the possibility.

@tacaswell tacaswell modified the milestones: v3.2.0, v3.3.0 Aug 27, 2019
@QuLogic QuLogic modified the milestones: v3.3.0, v3.4.0 May 2, 2020
@jklymak jklymak marked this pull request as draft May 6, 2020 19:50
@jklymak
Copy link
Member Author

jklymak commented May 6, 2020

Taking this up again in a simpler form in #17347

@jklymak jklymak closed this May 6, 2020
@jklymak jklymak deleted the enh-reuse-gridspec-if-possible branch May 6, 2020 23:59
@tacaswell tacaswell modified the milestones: v3.4.0, unassigned Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants