Fix label_outer in the presence of colorbars. #30098
Conversation
| return | ||
| gs = ss.get_gridspec() | ||
| if (isinstance(gs, mpl.gridspec.GridSpecFromSubplotSpec) | ||
| and gs.nrows * gs.ncols == 6): |
There was a problem hiding this comment.
Please add a comment what this magic is about.
There was a problem hiding this comment.
Thanks. Basically ok.
Would it make more sense that make_axes_gridspec explicitly flags the gridspec? Not sure what the right level of explicitness / specialization is, but it feels awkward to detect a gridspec for the colorbar thrugh nrows * ncols. Options could be (from less to more formal):
- monkey-patch
_is_colorbar_gridspeconto the gridspec, so that we can testif hasattr(gs, "_is_colorbar_gridspec") - add a
_is_colorbar_gridspecattribute to all gridspecs, so that we can testif gs._is_colorbar_gridspec - Create a
ColorbarGridSpecsubclass so that we can testif isinstance(gs, ColorbarGridSpec)
There was a problem hiding this comment.
I would lean towards the least formal here - the subgridspec trick of making colorbars is pretty clunky, and note that it is explicitly not used by layout='constrained'.
The original ideas behind grid specs and subgridspecs were very good, but unfortunately loses the relationship between sibling axes if you change add the colorbar and make the colorbar and main axes child subgridspecs.
The subgridspec to be considered should be the one containing both the axes and the colorbar, not the sub-subgridspec of just the axes.
| f, axs = plt.subplots(2, 2, sharex=True, sharey=True) | ||
| @pytest.mark.parametrize('with_colorbar', [True, False]) | ||
| def test_label_outer(remove_ticks, with_colorbar): | ||
| fig, axs = plt.subplots(2, 2, sharex=True, sharey=True) |
There was a problem hiding this comment.
Can this get tested with layout=constrained as well?
The subgridspec to be considered should be the one containing both the axes and the colorbar, not the sub-subgridspec of just the axes.
Closes #27305 (using #27305 (comment) and a test).
PR summary
PR checklist