Skip to content

Support subplots((m, n), ...) as shorthand for subplots(m, n, squeeze=False, ...) #19463

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

Closed
anntzer opened this issue Feb 5, 2021 · 11 comments
Closed
Labels
New feature status: inactive Marked by the “Stale” Github Action status: needs comment/discussion needs consensus on next step

Comments

@anntzer
Copy link
Contributor

anntzer commented Feb 5, 2021

Problem

subplots() defaults to auto-squeezing the returned Axes array (returning a single array when one is requested, returning a 1D array when a single row or single column is requested), likely for practicality, but this makes dynamic layouts pretty annoying: if you write axs = fig.subplots(m, n) where m and n are somehow dynamically computed (based on some dataset characteristics) and then later plan to use axs[i, j], you basically have an IndexError waiting to happen unless you can be certain that m and n are never 1. Of course one can remember to always write subplots(..., squeeze=False), but that's a bit verbose.

Proposed Solution

My preferred solution would probably have been to make the default m and n not 1, but None, so that None means "1 in that direction, and squeeze that direction" whereas 1 just means 1 (without squeezing). This would have fixed most common cases, e.g. if you write subplots() you get a single axes, subplots(3) or subplots(nrows=3) or subplots(ncols=3) you get a 1D array, subplots(m, n) (where m and n are dynamically computed variables) you always get a 2D array. I think in practice the main back-incompatibility of this proposal that occurs in real life is subplots(1, 3) (single row, likely not so uncommon), which would typically have to be written subplots(None, 3) or subplots(ncols=3) or fig, (axs,) = subplots(3) (for the additional unpacking).

Assuming that this backcompat breakage is not acceptable (it likely is not), one alternative would be to change the signature of subplots() to also support taking a single 2-tuple as argument (rather than nrows and ncols separately, and make subplots((m, n)) never squeeze the input. (Likely we wouldn't bother supporting m or n being None, in that case.) Then it would be OK to retrain my muscle memory to just always add an extra pair of parentheses when calling subplots.

Additional context and prior art

@tacaswell tacaswell added this to the v3.5.0 milestone Feb 5, 2021
@jklymak
Copy link
Member

jklymak commented Feb 5, 2021

I empathize with the overall problem and this isn't a terrible solution. But I prefer your initial proposal and wonder if we could find a way to deprecate the auto-squeeze for subplot(1,3).

@anntzer anntzer added the status: needs comment/discussion needs consensus on next step label Feb 5, 2021
@anntzer
Copy link
Contributor Author

anntzer commented Feb 5, 2021

Let's collect other devs' opinions for now.

@timhoffm
Copy link
Member

timhoffm commented Feb 6, 2021

The current API of subplots() is not that bad

I hear your concerns but want to question the size of the problem. IMHO squeezing is the right thing for interactive use and probably the most common calls are subplots(2, 1) and subplots(1, 2) (or subplots(ncols=2)) where you don't want to bother with two indices. Dynamic layouts where where m and n are not known and can be 1 seem rather a niche application and more advanced. subplots(..., squeeze=False) is bearable verbosity in such a case. I argue that the current API is mostly reasonable here.

There's no reasonable way to tune the signature of subplots()

Even if there are slightly better signatures, there's no clean and backward-compatible change we can do. The None concept would be good but is a prohibitive compatibility breakage. subplots((m, n)) makes the signature fuzzy and hard to document, either you have to accept subplots(nrows=(m, n)) as well or you end up with def subplots(*args).

But we may want to return a custom AxesGroup object

A possible improvement could be moving away from ndarray and providing our own AxesGroup (name up to bikeshedding). This has a number of advantages:

  • We're somewhat mis-using ndarray as a grid container. AFAICS the only reasonable operations are index access and iterating though all elements. Arithmetics, changing shape etc. do no not have a meaning anyway. A dedicated container could be more clear.

  • We are completely free in defining the indexing:

    • If the layout is only one column or one row, we can simultaneouly accept axs[n] as well as axs[1, n] / axs[n, 1]. This would basically make the squeeze parameter obsolete (An edge case is fig, ax =plt.subplots(). I have not thought it through, but that might warrant keeping squeeze around).
    • One could (not sure if one should) even accept an overall running index, e.g. for subplots(2, 3)
      0 1 2
      3 4 5
      
      axs[4] would be equivalent to axs[1, 1]. Note that is almost equivalent to the index k in subplot(m, n, k), but would start counting from 0 instead 1.
  • Similar to the Spines container (Add Spines class as a container for all Axes spines #17107), we could use such a class as proxy for batch operations, e.g. you could do

    • axs.set_axis_off() to turn all all axis simultaneously or
    • axs[:,0].set_ylabel('foo') to set the ylabels on all left-edge plots.

    This needs careful design but I'm positive that it can be done (see the Spines).

Concerning backward-compatibility: We cannot (actually could but don't want to) support all the ndarray API, e.g. somebody could theoretically have done np.rollaxis(axs, 0). We can however make it compatible for the reasonable use cases, e.g. supporting axs.shape, axs.flat etc.

@anntzer
Copy link
Contributor Author

anntzer commented Feb 6, 2021

IMHO squeezing is the right thing for interactive use and probably the most common calls are subplots(2, 1) and subplots(1, 2) (or subplots(ncols=2)) where you don't want to bother with two indices.

The first one I nearly always spell subplots(2) anyways, which is shorter, and leaves the second argument at its default (None), so we can differentiate that. The real "problem" case is indeed subplots(1, n), for which subplots(ncols=n) exists but is likely less often used.

Dynamic layouts where where m and n are not known and can be 1 seem rather a niche application and more advanced. subplots(..., squeeze=False) is bearable verbosity in such a case.

I can only say that I run into this problem quite often :(

There's no reasonable way to tune the signature of subplots()

I think subplot(*args) is actually quite reasonable. Right now the docstring for nrows, ncols is actually

nrows, ncols : int, default: 1
    Number of rows/columns of the subplot grid.

so that could easily be turned into

args : up to two ints, or a pair of ints
    Number of rows and columns of the subplot grid.  If a single int *n*, it is interpreted as ``(n, 1)``.

(and yes we'd need to allow grabbing nrows, ncols out of **kwargs but I'd deem that a backcompat detail and just not bother documenting it).

a custom AxesGroup object

Actually I do like the idea too, but that would be quite a bit more work to implement.

@timhoffm
Copy link
Member

timhoffm commented Feb 6, 2021

(and yes we'd need to allow grabbing nrows, ncols out of **kwargs but I'd deem that a backcompat detail and just not bother documenting it).

The documentation must still cover subplots(ncols=2). It's a valid and I'd even say recommended API. And then you have to decide what to do if args and kwargs are present and implement it.

On a more general note, I don't quite like having redundant func(x, y) and func((x, y)) API to begin with. And having them with subtly different semantic makes it worse. That's too clever and not intuitive. If somebody reads the code they will not be able to tell that it makes a difference without reading the docs. And even if you tell somebody these two calls do different things, they still would not in any way be able to guess what the difference is. IMO that's a clear sign of a bad API.

Actually I do like the idea too, but that would be quite a bit more work to implement.

If for a start we just make it a grid container, that is largely compatible with ndarray that should be fairly easy. I should be able to write that down in an hour or two. More features like batch operations can be added later.

@andrzejnovak
Copy link
Contributor

Just wanted to drop a comment if the signature ends up reworked one way or another that the fact that it's (rows, columns), but figsize is (x, y) aka (column_size, row_size) is moving me a touch closer to madness every time I type it.

@jklymak
Copy link
Member

jklymak commented Feb 6, 2021

Graphics are almost always specified width times height. Matrices are almost always specified rows times columns. You can argue its internally inconsistent, but it is consistent with long-standing conventions.

@tacaswell
Copy link
Member

In retrospect, not flipping the default of squeeze when we pulled subplots() up to Figure from pyplot was a missed opportunity.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Oct 6, 2023
@timhoffm
Copy link
Member

timhoffm commented Oct 6, 2023

Even though we have no agreed on an alternative/better solution, I propose to close this as not intuitive and too clever.
Nevertheless, thanks @anntzer for trying to come up with a solution to the awful squeeze problem.

@anntzer anntzer closed this as completed Oct 6, 2023
@anntzer
Copy link
Contributor Author

anntzer commented Oct 6, 2023

The idea stays here anyways, I agree this doesn't need to start open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New feature status: inactive Marked by the “Stale” Github Action status: needs comment/discussion needs consensus on next step
Projects
None yet
Development

No branches or pull requests

6 participants