Skip to content

Regression in add_subplot.. #17343

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
jklymak opened this issue May 6, 2020 · 8 comments · Fixed by #17350
Closed

Regression in add_subplot.. #17343

jklymak opened this issue May 6, 2020 · 8 comments · Fixed by #17350
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone

Comments

@jklymak
Copy link
Member

jklymak commented May 6, 2020

#16527 419fafe added checks for add_subplot that break:

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()

Old behaviour

Figure_1

New behaviour:

  ax2 = fig.add_subplot(3, 2, (3, 5))
Traceback (most recent call last):
  File "testPlots.py", line 6, in <module>
    ax2 = fig.add_subplot(3, 2, (3, 5))
  File "/Users/jklymak/matplotlib/lib/matplotlib/figure.py", line 1376, in add_subplot
    args = tuple(map(int, args))
TypeError: int() argument must be a string, a bytes-like object or a number, not 'tuple'
@jklymak jklymak added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label May 6, 2020
@jklymak jklymak added this to the v3.3.0 milestone May 6, 2020
@jklymak
Copy link
Member Author

jklymak commented May 6, 2020

This hasn't been released yet, so should get a fix before 3.3 goes in (or be reverted)

@story645
Copy link
Member

story645 commented May 6, 2020

did not know you could do axes spanning via add_subplot - is this a thing we want to advertise?

@jklymak
Copy link
Member Author

jklymak commented May 6, 2020

Well, I don't think we can break it, and it seems a useful feature. Note that this usage is also compatible with matlab's

@efiring
Copy link
Member

efiring commented May 7, 2020

This must rank among the ugliest carry-overs from the early attempt at Matlab compatibility. Can we at least document it as such--"un-Pythonic, unreadable, not recommended"?

@jklymak
Copy link
Member Author

jklymak commented May 7, 2020

Hmmm. Well I don’t know if the other ways of doing the same thing are so great that this deserves so much approbation. Making a gridspec and passing slices to add_subplot is a bit convoluted.

@timhoffm
Copy link
Member

timhoffm commented May 8, 2020

Wait, what?!? It took me 5 minutes to find out what fig.add_subplot(3, 2, (3, 5)) is supposed to do. This is far more crazy than subplot(325) (personal opinion).

I wouldn't be opposed deprecating it. Apparently we did neither have documentation nor tests. But if this get's documented instead, we should note that this is for MATLAB compatibility and maybe discourage it's use. IMHO We shouldn't judge our APIs (un-pythonic/unreadable).

@efiring
Copy link
Member

efiring commented May 8, 2020

Well, yes, I wasn't seriously proposing wording for our documentation. I would support deprecation in some form. I think #16603 is the basis for a better API for typical uses of add_subplot, but it doesn't include the ability of add_subplot to add one subplot at a time, potentially overlapping each other.

Maybe we need to think a little more about exactly what APIs we want to end up with in the long run. As time goes on, Matlab compatibility becomes less useful, and more of a burden.

@jklymak
Copy link
Member Author

jklymak commented May 8, 2020

The Matplotlib alternatives to ax = fig.add_subplot(3, 2, (3, 5)) are:

gs = fig.add_gridspec(3, 2)
ax = fig.add_subplot(gs[1:, 0])

or ax = plt.subplot2grid(3, 2, (1, 0), rowspan=2, fig=fig).

I substantially prefer the gridspec method of the three, but I don't consider any of them particularly awesome to justify deprecating any of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
4 participants