Skip to content

FIX: allow start-stop subplot#17344

Closed
jklymak wants to merge 2 commits intomatplotlib:masterfrom
jklymak:fix-allow-start-stop-subplot
Closed

FIX: allow start-stop subplot#17344
jklymak wants to merge 2 commits intomatplotlib:masterfrom
jklymak:fix-allow-start-stop-subplot

Conversation

@jklymak
Copy link
Copy Markdown
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
Copy Markdown
Member

tacaswell commented May 6, 2020

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

@jklymak
Copy link
Copy Markdown
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
Comment thread lib/matplotlib/figure.py Outdated
Comment thread lib/matplotlib/figure.py Outdated
Comment thread lib/matplotlib/figure.py Outdated
QuLogic
QuLogic reviewed May 6, 2020
View reviewed changes
Comment thread lib/matplotlib/figure.py Outdated
@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
Comment thread lib/matplotlib/figure.py Outdated
*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
Copy Markdown
Member

@QuLogic QuLogic May 7, 2020

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
Copy Markdown
Contributor

anntzer commented May 7, 2020

See #17350 for an alternative approach.

@jklymak
Copy link
Copy Markdown
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
Copy Markdown
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