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: Subfigures #18356

Merged
merged 1 commit into from
Oct 9, 2020
Merged

ENH: Subfigures #18356

merged 1 commit into from
Oct 9, 2020

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Aug 25, 2020

PR Summary

This PR re-introduces subfigures:

import matplotlib.pyplot as plt
import numpy as np

def example_plot(ax, fontsize=12, hide_labels=False):
    pc = ax.pcolormesh(np.random.randn(30, 30))
    if not hide_labels:
        ax.set_xlabel('x-label', fontsize=fontsize)
        ax.set_ylabel('y-label', fontsize=fontsize)
        ax.set_title('Title', fontsize=fontsize)
    return pc


# gridspec inside gridspec
fig = plt.figure(constrained_layout=True, figsize=(10, 4))
subfigs = fig.subfigures(1, 2, wspace=0.07)

axsLeft = subfigs[0].subplots(1, 2, sharey=True)
subfigs[0].set_facecolor('0.75')
for ax in axsLeft:
 pc = example_plot(ax)
subfigs[0].suptitle('Left plots', fontsize='x-large')
subfigs[0].colorbar(pc, shrink=0.6, ax=axsLeft, location='bottom')

axsRight = subfigs[1].subplots(3, 1, sharex=True)
for nn, ax in enumerate(axsRight):
 pc = example_plot(ax, hide_labels=True)
 if nn == 2:
     ax.set_xlabel('xlabel')
 if nn == 1:
     ax.set_ylabel('ylabel')
subfigs[1].colorbar(pc, shrink=0.6, ax=axsRight)
subfigs[1].suptitle('Right plots', fontsize='x-large')

fig.suptitle('Figure suptitle', fontsize='xx-large')

plt.show()

Subpanles

Subfigures are created via: fig.subfigures (like fig.subplots) and fig.add_subfigure (like fig.add_subplots). Before these would have been added with GridspecFromSubgridspec objects and those objects would not have had the ability to have subtitles, or their own legends.

Subfigures are basically figures without the figure management. Anything you can add to a figure you can add to a sub-panel. That required significant refactoring in figure.py where all the artist methods in figure.py are moved FigureBase, and then SubFigurel and Figure are subclasses.

This is relatively major surgery to figure.py though the new Figure class should be exactly the same as the old one.

Test Changes:

  • test_polar.py: Increased tolerance for two annotation tests
  • test_plot_3d_from_2: increased tolerance from 0.01 to 0.015.
  • A few 3-d axes had: ax=Axes3D(fig); previously Axes3D.__init__ would add the axes to the figure (which is not an unreasonable expectation, but not one that is supported by Axes). This caused the axes to be added twice to the figure. So that extra fig.add_axes has been removed in Axes3D.__init__ and the two tests in the suite that used that grammar were changed to explicitly add the axes: ax=Axes3D(fig); fig.add_axes(ax).

PR Checklist

  • it fails the image tests with colorbars for some unknown reason. I'll track that down.
  • It needs more documentation
  • it needs tests
  • use this for subplot_mosaic?

@jklymak jklymak added topic: geometry manager LayoutEngine, Constrained layout, Tight layout status: needs comment/discussion needs consensus on next step labels Aug 25, 2020
@timhoffm
Copy link
Member

timhoffm commented Aug 26, 2020

I haven't yet looked into the details, but the overall idea seems reasonable. What is the motivation to name this "SubPanel"? IMHO just "Panel" would be simpler and is equally expressive.

@jklymak
Copy link
Member Author

jklymak commented Aug 26, 2020

subpanel was meant to be evocative of subplot, which this is directly analogous to. i.e the subpanel slots into a figure exactly the same way a subplot does. Not completely against the idea of just calling it a panel, but it really is meant to be a subordinate part of a larger figure, so I think sub makes sense.

One thing I should have mentioned above is that this is recursive. i.e. a subpanel can be added inside a subpanel, etc etc. So @QuLogic was making slides with matplotlib - this would have been a lot easier if he'd had subpanels to work with.

@timhoffm
Copy link
Member

https://en.wikipedia.org/wiki/Panel#Visual_arts

Panel (comics), a single image in a comic book, comic strip or cartoon [...]
Panel painting, in art, either one element of a multi-element piece of art, such as a triptych [...]

To me, there's enough "sub" notion in panel already without explicitly naming it.

@jklymak
Copy link
Member Author

jklymak commented Aug 26, 2020

I agree with you that a panel can be a subdivision. Or it can be a single thing.

https://www.merriam-webster.com/dictionary/panel
https://en.wikipedia.org/wiki/Panel#Science_and_technology

The argument for adding sub is to make it rhyme with subplots, where the sub is also arguably redundant. So right now you can do:

fig = plt.figure()
gs = fig.add_gridspec(2, 1)

# LHS
ax = fig.add_subplot(gs[0])

#RHS
panel = fig.add_subpanel(gs[1])
subaxs = panel.subplots(2,2)

Originally, it was subfigure, but somewhere along the line that got rejected...

@tacaswell
Copy link
Member

I am very 👍 on this in principle.

@jklymak
Copy link
Member Author

jklymak commented Sep 16, 2020

Ok, so I moved a bunch of methods from Figure.foo to PanelBase.foo, and Figure is now a subclass of PanelBase. Which shouldn't matter, except Sphinx autosummary :: can no longer find Figure.foo and says reference target not found: Figure.foo. I don't think we want to change every instance of Figure.foo, though I guess I could. Is there no way to get sphinx to allow the subclass to reference its parent?

@mwaskom
Copy link

mwaskom commented Sep 16, 2020

@jklymak
Copy link
Member Author

jklymak commented Sep 16, 2020

Yes Absolutley. I already have a pr for that but dont want to rebase it all the time.

@mwaskom
Copy link

mwaskom commented Sep 16, 2020

Awesome! Also, regarding the name, I feel like when discussing figures in journal articles, "panel" is the term often used for what in matplotlib would be a called a subplot or axes, e.g. "Panel (A) shows the mean frobexity over time, Panel (B) shows blah blah blah". Having "subpanel" be concept that is actually super-ordinate to that object might be confusing...

@jklymak
Copy link
Member Author

jklymak commented Sep 16, 2020

Originally, it was just subfigure - I've lost where it was discussed and rejected as a name, but I'd actually prefer it as the inheritance would then be Figure(FigureBase) and SubFigure(FigureBase) (ahem, plus or minus the currently unfashionable camel-case).

Edit: but open to other names as well...

@jklymak
Copy link
Member Author

jklymak commented Sep 17, 2020

Thanks to some help I got the docs to build. Thanks @larsoner. This does change the figure_api.rst to be monolithic however, as that was how I was able to get the inherited members to build properly. Anyone with better experience with autodoc is welcome to subsequently re-organize.

This needs input for the name, otherwise ready-is to go.... subpanel, panel, subfigure, others???

@jklymak
Copy link
Member Author

jklymak commented Sep 17, 2020

As pointed out by @story645 on gitter, subfigure is also used in Latex subfig package....

@jklymak
Copy link
Member Author

jklymak commented Sep 17, 2020

There is a user-contributed matlab subfigure, but it is a window tiler (not something I think we should have ;-) https://www.mathworks.com/matlabcentral/fileexchange/23426-subfigure

@tacaswell tacaswell added this to the v3.4.0 milestone Sep 21, 2020
@jklymak
Copy link
Member Author

jklymak commented Sep 21, 2020

On call we decided that subfigure should be the new name.

Also noted that need to make sure Annotate and ConnectionPatch work on subFigure.

@jklymak jklymak changed the title ENH: Subpanels ENH: Subfigures Sep 21, 2020
@jklymak jklymak force-pushed the enh-subpanels branch 2 times, most recently from 9f6e0e6 to 1dd462d Compare September 21, 2020 23:06
@jklymak jklymak removed the status: needs comment/discussion needs consensus on next step label Sep 21, 2020
@jklymak jklymak force-pushed the enh-subpanels branch 2 times, most recently from 7e91ab1 to 5752a94 Compare September 21, 2020 23:48
@jklymak
Copy link
Member Author

jklymak commented Sep 26, 2020

Is it worth adding an example that mixes Axes and SubFigures at the same level in the host Figure?

Example was expanded. Which was good, because I had a bug in the suptitle placement code....

@jklymak jklymak force-pushed the enh-subpanels branch 2 times, most recently from 8a73a4b to 835175b Compare September 30, 2020 05:03
@jklymak
Copy link
Member Author

jklymak commented Sep 30, 2020

@tacaswell Just one main enquiry for you about the compositing of images... Not sure I understood the comment.

@jklymak jklymak force-pushed the enh-subpanels branch 5 times, most recently from 454f97f to e373a36 Compare October 4, 2020 15:16
Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

My review is somewhat cursory and raises only few minor questions.

axsLeft = subfigs[0].subplots(1, 2, sharey=True)
subfigs[0].set_facecolor('0.75')
for ax in axsLeft:
pc = example_plot(ax)
Copy link
Member

Choose a reason for hiding this comment

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

Indentation is off.

is used and `.projections.polar.PolarAxes` if polar projection
is used. The returned axes is then a subplot subclass of the
are used and `.projections.polar.PolarAxes` if polar projection
are used. The returned axes is then a subplot subclass of the
Copy link
Member

Choose a reason for hiding this comment

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

The "are" instances were correct before as "is".

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats a strange one. I don't think I changed that. Maybe I missed it in a rebase? Anyhow, fixed....

for examples. (Note: does not work with `add_subplot` or
`~.pyplot.subplot2grid`.)
"""
def __init__(self):
super().__init__()
# remove the non-figure artist _axes property
# as it makes no sense for a figure to be _in_ an axes
# this is used by the property methods in the artist base class
# which are over-ridden in this class
Copy link
Member

Choose a reason for hiding this comment

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

Why did you leave the comment, but move the deletion to the subclass? Doesn't the deletion make sense as a base class operation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. re-added the deletion in the base class and removed it from class Figure.

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

It is difficult to find a place to comment on it, but the legend docstring seems to be missing the Call signatures, the 3 examples of them, and the See Also section.

doc/api/next_api_changes/development/18356-JMK.rst Outdated Show resolved Hide resolved
doc/users/next_whats_new/subfigures.rst Show resolved Hide resolved
examples/subplots_axes_and_figures/gridspec_nested.py Outdated Show resolved Hide resolved
lib/matplotlib/_constrained_layout.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
@jklymak
Copy link
Member Author

jklymak commented Oct 7, 2020

Thanks @QuLogic - I still have to look at the legend issue, so I'll mark as draft until I sort that out.

@jklymak jklymak marked this pull request as draft October 7, 2020 20:04
@jklymak
Copy link
Member Author

jklymak commented Oct 8, 2020

OK think I fixed up legend: https://47262-1385122-gh.circle-artifacts.com/0/doc/build/html/api/figure_api.html#matplotlib.figure.Figure.legend Again, I must have missed that in a rebase...

@jklymak jklymak marked this pull request as ready for review October 8, 2020 01:56
Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

Hopefully last round, I think.

lib/matplotlib/figure.py Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Show resolved Hide resolved
def get_size_inches(self):
return self._parent.get_size_inches()

def get_constrained_layout(self):
Copy link
Member

Choose a reason for hiding this comment

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

These two could get a docstring, then?

lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
lib/matplotlib/figure.py Outdated Show resolved Hide resolved
@jklymak jklymak force-pushed the enh-subpanels branch 2 times, most recently from 10da45d to f542c0b Compare October 8, 2020 16:30
Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

One remaining typo.

lib/matplotlib/figure.py Outdated Show resolved Hide resolved
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
@QuLogic QuLogic merged commit 3b1be53 into matplotlib:master Oct 9, 2020
@QuLogic
Copy link
Member

QuLogic commented Oct 9, 2020

OK, let's see how this goes...

@jklymak jklymak deleted the enh-subpanels branch October 9, 2020 13:19
@jklymak
Copy link
Member Author

jklymak commented Oct 9, 2020

Thanks a lot @QuLogic, @efiring and @tacaswell!

@QuLogic, hopefully this helps when you make your next all-matplotlib slide deck ;-)

@tacaswell
Copy link
Member

🎉

I swear reviewing this was on my todo list....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants