Skip to content

FIX: allow start-stop subplot #17344

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

FIX: allow start-stop subplot #17344

wants to merge 2 commits into from
+82 −55

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented May 6, 2020 •
edited
Loading

PR Summary

Closes #17343

add_subplot is meant to accept spans for the third parameter, but this was broken by #16527 which was doing more careful argument checking. Happy to take criticism on how I did this, but I think it works...

import matplotlib.pyplot as plt

fig = plt.figure()

ax = fig.add_subplot(3, 4, 1)
ax2 = fig.add_subplot(3, 2, (3, 5))
plt.show()

Figure_1

TODO:

  • Add tests so this doesn't break again
  • Add examples?
  • Fix the docstring to make this clear

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

@jklymak jklymak added this to the v3.3.0 milestone May 6, 2020
@jklymak jklymak added API: argument checking Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels May 6, 2020
@jklymak jklymak requested a review from timhoffm May 6, 2020 22:04
@jklymak jklymak force-pushed the fix-allow-start-stop-subplot branch from a2f73b1 to 37593c4 Compare May 6, 2020 22:05
@tacaswell
Copy link
Member

I'm with @story645 's comment in the original issue "we support that ?!?". Agree that we should not break it.

@jklymak
Copy link
Member Author

jklymak commented May 6, 2020

matlab documentation: https://www.mathworks.com/help/matlab/ref/subplot.html See the subhead under "p"

Grid position for the new axes, specified as a scalar or vector of positive integers.

If p is a scalar positive integer, then subplot creates a subplot in grid position p.
If p is a vector of positive integers, then subplot creates a subplot that spans the grid positions listed in p.
Example: subplot(2,3,1) creates a subplot in position 1.

Example: subplot(2,3,[2,5]) creates a subplot spanning positions 2 and 5.

Example: subplot(2,3,[2,6]) creates a subplot spanning positions 2, 3, 5, and 6.

QuLogic
QuLogic reviewed May 6, 2020
View reviewed changes
QuLogic
QuLogic reviewed May 6, 2020
View reviewed changes
@jklymak jklymak mentioned this pull request May 6, 2020
Merged
6 tasks
@jklymak jklymak force-pushed the fix-allow-start-stop-subplot branch from 89fa0e0 to be1b730 Compare May 7, 2020 00:02
QuLogic
QuLogic approved these changes May 7, 2020
View reviewed changes
lib/matplotlib/figure.py Outdated
The position of the subplot described by one of

- Three integers (*nrows*, *ncols*, *index*). The subplot will
take the *index* position on a grid with *nrows* rows and
*ncols* columns. *index* starts at 1 in the upper left corner
and increases to the right.
and increases to the right. *index* can also be a two-tuple
specifying the (*start*, *stop*) indices of the subplot, i.e.,
Copy link
Member

Choose a reason for hiding this comment

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

Oops, just realized this is an e.g., not i.e..

Suggested change
specifying the (*start*, *stop*) indices of the subplot, i.e.,
specifying the (*start*, *stop*) indices of the subplot, e.g.,

@anntzer anntzer mentioned this pull request May 7, 2020
Merged
6 tasks
@anntzer
Copy link
Contributor

anntzer commented May 7, 2020

See #17350 for an alternative approach.

@jklymak
Copy link
Member Author

jklymak commented May 7, 2020

Closing in lieu of #17350 (though I would somewhat prefer to get a request to revise than a completely different PR that does the same thing).

@jklymak jklymak closed this May 7, 2020
@jklymak jklymak deleted the fix-allow-start-stop-subplot branch May 7, 2020 16:21
@anntzer
Copy link
Contributor

anntzer commented May 7, 2020

I generally try to do that, but here the whole approach was so different that I though it would be faster to just write it rather than to describe what I'd prefer. Sorry about that, wasn't trying to PR-snipe you :-)

@QuLogic QuLogic modified the milestones: v3.3.0, unassigned Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@timhoffm timhoffm Awaiting requested review from timhoffm

Assignees
No one assigned
Labels
API: argument checking Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression in add_subplot..
4 participants
@jklymak @tacaswell @anntzer @QuLogic