ENH: accept 'all'/'row'/'col' axis sharing in subplot_mosaic - #32239
ENH: accept 'all'/'row'/'col' axis sharing in subplot_mosaic#32239aimldlds wants to merge 1 commit into
Conversation
Figure.subplot_mosaic only accepted booleans for sharex and sharey, while Figure.subplots also takes 'none', 'all', 'row' and 'col'. Accept the string spellings in the mosaic API too, so the two APIs agree. A mosaic Axes may span several rows or columns, so it is attributed to every row or column it covers and spanning Axes transitively merge the groups they bridge. Grouping uses cbook.Grouper, and nested mosaics are resolved to their outer cell via SubplotSpec.get_topmost_subplotspec, so nested layouts are handled rather than rejected. Refs matplotlib#18305.
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process. You can also join us on discourse chat for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
|
Hi @aimldlds
Could you say something about how you settled on this rule? Do you have a specific use case in mind? |
|
Without having thought too much about this: I would have intuitively settled on a different rule: Axes with the same span form a group. So for sharey="row" and the groups are AB, CD, EF. The motivation is that shared axes are mostly used to make values in different axes visually comparable by aligning data values. If the sizes of the Axes differ, e.g. between B and C, that visual alignment of data values is lost and thus there is little reason to lock the ranges. |
|
Yes that would be my expectation as well. |
PR summary
subplot_mosaic only took True/False for sharex/sharey, but subplots also takes 'none'/'all'/'row'/'col'. PR #20107 added the booleans in 2021 and left the strings for later. This adds them.
The open question is what 'row'/'col' mean when an Axes spans several cells. The rule here: an Axes belongs to every row/column it covers, so spanning Axes transitively merge the groups they bridge. In 'AAE;C.E', where E spans both rows, sharex='row' groups A, C and E together, while sharex='col' gives {A, C} and {E}.
Grouping uses cbook.Grouper (the union-find already backing _shared_axes), and nested mosaics resolve to their outer cell via SubplotSpec.get_topmost_subplotspec, so nested layouts are handled rather than rejected.
I would value maintainer feedback on the spanning-Axes semantics - it is the reason this part of #18305 stayed open, and I am happy to change the rule.
Refs #18305.
AI Disclosure
AI (Claude) was used substantially in this PR: it explored the codebase, proposed the spanning-Axes rule, and drafted the implementation, the tests and the documentation. I reviewed the result and directed the choices, but I did not write the code myself.
Verification so far: the 40 existing TestSubplotMosaic tests plus the 9 added here pass, and ruff is clean - but that run was against a released matplotlib binary with this patch applied, not against a local build of this branch. I have not yet built matplotlib locally, so CI here is the first full test of the actual tree.
I am aware of the project's policy on AI use in good-first-issues. I am disclosing fully so maintainers can judge whether this is useful to them, and I will accept the outcome either way.
PR quality check
Use an expressive title, e.g. "Fix title font property precedence"
New and changed code is tested (9 new tests; see caveat in AI Disclosure)
N/A Plotting related features are demonstrated in an example
New features and API changes have release notes
Documentation complies with general and docstring guidelines