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]: gridspec_mosaic #24571

Closed
anntzer opened this issue Nov 30, 2022 · 13 comments · Fixed by #24604
Closed

[ENH]: gridspec_mosaic #24571

anntzer opened this issue Nov 30, 2022 · 13 comments · Fixed by #24604
Labels
New feature topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Milestone

Comments

@anntzer
Copy link
Contributor

anntzer commented Nov 30, 2022

Problem

Trying to combine subplot_mosaic with axes using various different projections (e.g. one rectilinear axes and one polar axes and one 3d axes) has been requested a few times (once in the original subplot_mosaic thread IIRC, and in #20392 too), and it's something I would recently have been happy to have, too.

Pushing projections directly into subplot_mosaic seems ripe for API bloat, but perhaps another solution would be to add figure.gridspec_mosaic(...) which takes the same arguments as subplot_mosaic, but returns a dict of subplotspecs, such that one can do something like

specs = fig.gridspec_mosaic(...)
d = {
    "foo": fig.add_subplot(specs["foo"], projection=...),
    "bar": fig.add_subplot(specs["bar"], projection=...),
    ...
}

As a side point, I do find the repetition of fig in each call to add_subplot a bit jarring (as the underlying gridspec is bound to the figure, so that information is actually redundant). Back in #13280 I had proposed to add SubplotSpec.add_subplot() (adding to the figure to which the gridspec is bound), which would allow one to write, here,

specs = fig.gridspec_mosaic(...)
d = {
    "foo": specs["foo"].add_subplot(projection=...),
    "bar": specs["bar"].add_subplot(projection=...),
    ...
}

but that idea got shot down back then and even if we decide not to revisit it, even the first form would be nice to have.

Thoughts?

Proposed solution

No response

@anntzer anntzer added New feature topic: geometry manager LayoutEngine, Constrained layout, Tight layout labels Nov 30, 2022
@story645
Copy link
Member

I like this better than the current create/remove/replace scheme, but just to be clear- using this method means folks would have to go in manually and create a subplot for each spec, right? So this feature is just providing the ability to layout and identify axes in the same way as subplot_mosaic?

@jklymak
Copy link
Member

jklymak commented Nov 30, 2022

We could do this, but it seems relatively convoluted, and likely to end up in a dusty corner... Isn't the fundamental problem that we can't change projections post-facto? Can we not do that somehow? ax.change_projection() would be best, but axnew = fig.change_projection(ax, proj) would perhaps be bearable.

@story645
Copy link
Member

and likely to end up in a dusty corner

I think that the loops for adding a new subplot would probably look almost identical to the ones for changing the projection, so I'm not sure that the discoverablity/documentation problem favors one over the other, especially if either solution is added to the mosaic tutorial and as a nice gallery example.

@tacaswell
Copy link
Member

The projection machinery can pick a different class to use for the returned Axes instance. While you can change classes in Python (which we do in mplot3d), I am not a huge fan of doing more of that to I think changing the projection is off the table.


Unfortunately we already pass subplot_kw to all of them and do not want to try to de-conflict the namespaces. Making something like:

from matplotlib import pyplot as plt

fig, axd = plt.subplot_mosaic(
    "AB;CC",
    needs_a_better_name={"A": {"projection": "polar"}, "B": {"projection": "3d"}},
)

work is not that bad (I used a bad name to avoid getting lost in discussions of the parameter name just yet ;) ).

so

diff --git a/lib/matplotlib/figure.py b/lib/matplotlib/figure.py
index 6c18ba1a64..e75b973044 100644
--- a/lib/matplotlib/figure.py
+++ b/lib/matplotlib/figure.py
@@ -1771,7 +1771,8 @@ default: %(va)s

     def subplot_mosaic(self, mosaic, *, sharex=False, sharey=False,
                        width_ratios=None, height_ratios=None,
-                       empty_sentinel='.', subplot_kw=None, gridspec_kw=None):
+                       empty_sentinel='.', subplot_kw=None, gridspec_kw=None,
+                       needs_a_better_name=None):
         """
         Build a layout of Axes based on ASCII art or nested lists.

@@ -1868,6 +1869,7 @@ default: %(va)s
         """
         subplot_kw = subplot_kw or {}
         gridspec_kw = dict(gridspec_kw or {})
+        needs_a_better_name = needs_a_better_name or {}
         if height_ratios is not None:
             if 'height_ratios' in gridspec_kw:
                 raise ValueError("'height_ratios' must not be defined both as "
@@ -2011,7 +2013,11 @@ default: %(va)s
                         raise ValueError(f"There are duplicate keys {name} "
                                          f"in the layout\n{mosaic!r}")
                     ax = self.add_subplot(
-                        gs[slc], **{'label': str(name), **subplot_kw}
+                        gs[slc], **{
+                            'label': str(name),
+                            **subplot_kw,
+                            **needs_a_better_name.get(name, {})
+                        }
                     )
                     output[name] = ax
                 elif method == 'nested':
diff --git a/lib/matplotlib/pyplot.py b/lib/matplotlib/pyplot.py
index 79c33a6bac..f384fe747b 100644
--- a/lib/matplotlib/pyplot.py
+++ b/lib/matplotlib/pyplot.py
@@ -1474,7 +1474,8 @@ def subplots(nrows=1, ncols=1, *, sharex=False, sharey=False, squeeze=True,

 def subplot_mosaic(mosaic, *, sharex=False, sharey=False,
                    width_ratios=None, height_ratios=None, empty_sentinel='.',
-                   subplot_kw=None, gridspec_kw=None, **fig_kw):
+                   subplot_kw=None, gridspec_kw=None,
+                   needs_a_better_name=None, **fig_kw):
     """
     Build a layout of Axes based on ASCII art or nested lists.

@@ -1571,7 +1572,8 @@ def subplot_mosaic(mosaic, *, sharex=False, sharey=False,
         mosaic, sharex=sharex, sharey=sharey,
         height_ratios=height_ratios, width_ratios=width_ratios,
         subplot_kw=subplot_kw, gridspec_kw=gridspec_kw,
-        empty_sentinel=empty_sentinel
+        empty_sentinel=empty_sentinel,
+        needs_a_better_name=needs_a_better_name,
     )
     return fig, ax_dict

@greglucas
Copy link
Contributor

Just to chime in and say that I have also had the desire to have a dropdown selector of geographic projections to click through/change at will. (A nice example here: https://observablehq.com/@d3/projection-transitions) If someone calls ax.change_projection() I think we could get away with some documentation that the class type may change underneath them..., and do some fiddling on our end to update the transforms and __class__ attributes properly (which is probably not trivial to get right).

@anntzer
Copy link
Contributor Author

anntzer commented Dec 1, 2022

Ah, I see that #20392 proposed to pass subplot_kws as an array of dicts (which is a bit of a mess once you have axes spanning multiple cells), and I can't find if a concrete proposal had been made in the original thread (#16603); but I agree that passing instead a dict of dicts (label -> subplot_kw) as proposed by @tacaswell seems not too bad.

@rcomer
Copy link
Member

rcomer commented Dec 1, 2022

My first follow-up feature request would be to do

from matplotlib import pyplot as plt

fig, axd = plt.subplot_mosaic(
    "AB;CC",
    needs_a_better_name={"AB": {"projection": "polar"}},
)

to make both the top panels polar. Sometimes you might want like 10 axes with 5 in one projection and 5 in another.

@timhoffm
Copy link
Member

timhoffm commented Dec 1, 2022

needs_a_better_name={"AB": {"projection": "polar"}},

I think the generic solution is

needs_a_better_name={
    ("name1", "name2"): {"projection": "polar"}},
    "name3": {"projection": "3d"},
}

i.e. keys are strings or tuples of strings.

Maybe with the extension: If the mosaic spec is a string (i.e. we only have single-char names), any key with len>1 is interpreted as tuple(key), i.e. "AB" is internally converted to ("A", "B").

@story645
Copy link
Member

story645 commented Dec 1, 2022

we already pass subplot_kw to all of them and do not want to try to de-conflict the namespaces

So which one would get priority if like subplot_kw and needs_a_better_name get passed the same key?

@ksunden
Copy link
Member

ksunden commented Dec 1, 2022

In Tom's diff, needs_a_better_name would get priority as it is later in the unpacking.

>>> {"a": 1, "a":2}
{'a': 2}

This makes intuitive sense as it allows you to set some generic settings for most subplots but override with the more specific kwargs for individual subplot, which you simply would not do if that had no effect.

@story645
Copy link
Member

story645 commented Dec 1, 2022

This makes intuitive sense

Yeah, it's just gonna have to be documented in neon lights and commented as this is a for free of python dict implementation. I'm worried though about two keywords that do almost the same thing but one is vectorized, since that feels like color vs colors

@tacaswell
Copy link
Member

Implementing @rcomer 's idea for the short hand was easy, but I have more concerns about the general case as we will technically work with any hashables as the keys so there is an ambiguity there. Not insurmountable, but annoying.

Pulling out a (maybe private) gridspec_mosaic may still be useful so we can implement subfigure_mosaic as well....

@tacaswell tacaswell added this to the v3.7.0 milestone Dec 3, 2022
@tacaswell
Copy link
Member

I have more concerns about the general case as we will technically work with any hashables as the keys so there is an ambiguity there.

I was wrong about that, trying to use tuples as keys blows up currently.

I do not think that there are any of the singular/plural keywords for the Axes family of classes, but if there are we are not exposing any ambiguity that is not there already and we can let the class do what ever they would normally do if the user did fig.add_subplot(gs, ...).

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

Successfully merging a pull request may close this issue.

8 participants