ENH: Added sharex/sharey string support to subplot_mosaic by nillohitroy · Pull Request #32437 · matplotlib/matplotlib

nillohitroy · 2026-10-04T14:48:52Z

PR summary

This PR implements string arguments ('all', 'row', 'col') and boolean True/False for sharex and sharey in subplot_mosaic() for #18305 .

To handle complex and nested layouts, the axes are grouped based on their specific GridSpec and exact rowspan/colspan coordinates. Axes only share a row or column if their spans are identical.
For example, consider a layout where C and D span both rows:

ABCD
EFCD

If sharey='row' is passed, the code forms three isolated sharing groups based on their vertical spans:

[A, B] (Row 0)
[E, F] (Row 1)
[C, D] (Spanning Rows 0 and 1)

Once the groups are isolated, the logic designates the first axis in each list as the parent and calls standard .sharex(parent) or .sharey(parent) on the remaining children in that specific group.

Important Note: Relying on Matplotlib's global ax._label_outer_xaxis() or ax._label_outer_yaxis() inadvertently hid tick labels for inner subplots if they formed their own isolated sharing group. For this very reason, I implemented a custom logic which calculates label visibility dynamically per group. For example, in an X-axis sharing group, it calculates the physical bottom edge by finding the max() of rowspan.stop across just the group's members. It then loops through that group and applies ax.tick_params(labelbottom=False) to any axis sitting above that localized bottom edge.

AI Disclosure

I have used Generative AI to understand the codebase (specifically the code for the issue), setting up of the environment (installing packages and dependencies required), the intent of the issue and to get to the center of the problem. It is also important to mention that AI was used to understand the requirements of the maintainers and how #32239 failed and what to avoid while coding.

Verification

I have tested the code for a local matplotlib development build, running it on my system and creating a sample plot for a sample data. Also, I have written some test cases for the same by trying to incorporate as many test cases as possible (5), all of which successfully passed.

I would be happy for any feedback regarding any portion of the code!!

PR quality check

Use an expressive title, e.g. "Fix title font property precedence"
New and changed code is tested
[/] Plotting related features are demonstrated in an example
[/ ] New features and API changes have release notes
[/ ] Documentation complies with general and docstring guidelines

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

github-actions · 2026-10-04T14:49:03Z

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.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

nillohitroy · 2026-10-06T14:59:20Z

Dear Maintainers,
I have seen how diligently you guys work and how mentally constraining reviewing each line of the code is. Though this issue is a 'good first issue' and not as complex (and may be not as important) as compared to the other bug fixes that the contributors are providing. I would be obliged if you could review my code and give me necessary feedback to improve it as much as possible.

iccir · 2026-10-06T18:14:38Z

Assuming that I'm talking to an actual human: this PR is currently failing to pass the Linting and MyPy Stubtest checks. The AppVeyor build failure is somewhat expected as AppVeyor has been flaky recently.

Generally, you will want to make sure that all checks pass on any PR that you open to a repository. You need to get at least Linting and the stub test to pass first.

nillohitroy · 2026-10-07T01:00:13Z

Assuming that I'm talking to an actual human: this PR is currently failing to pass the Linting and MyPy Stubtest checks. The AppVeyor build failure is somewhat expected as AppVeyor has been flaky recently.

Generally, you will want to make sure that all checks pass on any PR that you open to a repository. You need to get at least Linting and the stub test to pass first.

Thank you sir.. on it..

nillohitroy · 2026-10-07T16:35:19Z

Hi @iccir
I think I have screwed up in some way. Thing is, I have done a pull request and merged the code, so instead of just 1 commit, there is 11 commits and also some failed tests (of other developers). I have checked my tests (linting and all) and everything is successful. My question is: will everything work or do I have to open a new pull request?
Sorry for any trouble I have caused.

rcomer · 2026-10-07T16:46:52Z

I think a rebase should sort that out. Assuming you have set the upstream remote as described here, try this:

Make a backup branch in case something else goes wrong

git branch my-backup-branch

Rebase on the upstream branch

git fetch upstream
git rebase upstream/main

git push will then fail, but you can

git push --force-with-lease

iccir · 2026-10-07T16:50:19Z

Hey, it happens - git can be tricky :)

It looks like you made two commits:
c278400
a14df4f

I'd do something like the following:

git diff \
    c278400dee9b6758ce74f0eab9b5cbc085b595ee^ \
    a14df4f320b0b13ebdf7fc34cf4d356e5f0f67ea > \
    /someplace/safe/changes.patch

Then I'd follow @rcomer's instructions above. You can always reapply the patch with:

git apply /someplace/safe/changes.patch

nillohitroy · 2026-10-08T02:51:14Z

Hi @rcomer @iccir
Thank you for being so kind and helpful. I have solved the issue with the merge request.
However, as shown, there is a failing test with Mac OS and I don't have a MAC so I really don't know what the issue is.

iccir · 2026-10-08T03:08:46Z

If you get a test failure that you don't understand, there are several possibilities:

The test failure is caused by a specific version of Python. For example, we could see Python 3.12 failing across several different operating systems.
The test failure is caused by a specific platform. We could see all macOS builders fail, for example.
A test failure on a specific version of Python and a specific platform. You would see a single test failure, as in this case.

More commonly, however, #3 can indicate that something is wrong with the Continuous Integration (CI) system.

You can often look at other pull requests to see if they are having similar issues. Python 3.12 on macOS 15 has been failing all day - here's another example with the same error.

So, it's probably not an error in your code!

nillohitroy · 2026-10-08T05:13:08Z

If you get a test failure that you don't understand, there are several possibilities:

The test failure is caused by a specific version of Python. For example, we could see Python 3.12 failing across several different operating systems.

The test failure is caused by a specific platform. We could see all macOS builders fail, for example.

A test failure on a specific version of Python and a specific platform. You would see a single test failure, as in this case.

More commonly, however, #3 can indicate that something is wrong with the Continuous Integration (CI) system.

You can often look at other pull requests to see if they are having similar issues. Python 3.12 on macOS 15 has been failing all day - here's another example with the same error.

So, it's probably not an error in your code!

I also have the same view as you. It is showing that the test is failing only in macos-15 (macos26 is running successfully). Also, I have tested the code locally as well, so I'm pretty confident about it. My request is if you could review my code so as to ensure if there are other major issues or this code can be merged to the repo..

nillohitroy · 2026-10-10T10:33:54Z

Hi @rcomer
Sorry to disturb you like this. But is it possible for you to review my code?

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

🟡 Changes recommended

Fix nested-layout label visibility by comparing physical positions and update the public documentation.

3 open findings

🧠 Review effort: Lite

timhoffm · 2026-10-22T20:54:53Z

+                    bottom_edge = max(ax.get_subplotspec().rowspan.stop for ax in group)
+                    for ax in group:
+                        if ax.get_subplotspec().rowspan.stop < bottom_edge:
+                            ax.tick_params(labelbottom=False)


@nillohitroy The copilot suggestion to use physical coordinates is based on the assumption that grid nesting should not play a role. Have we decided that this is the case? For

["A", [["B", "C"], ["B", "D"]], ]

should A and B sharey? Are there realistic cases where that is necessary? It would be simpler to claim that col/row sharing across nested grids is not supported. The physical calculation is a bit annoying because you need tolerances.

rcomer · 2026-10-13T09:21:29Z

I'm sorry but I do not think I will be able to review this any time soon.

nillohitroy · 2026-10-13T14:23:41Z

I'm sorry but I do not think I will be able to review this any time soon.

I appreciate your efforts and I can understand it. I will figure it out in some way..

nillohitroy · 2026-10-13T14:25:06Z

Hi @iccir
Is it possible for you to review this PR? If so, can you please help me out?

timhoffm · 2026-10-14T15:11:58Z

+        #         ax.sharey(ax0)
+        #         ax._label_outer_yaxis(skip_non_rectangular_axes=True)
+
+        # Symmetric Sharing


Suggested change

# Symmetric Sharing

That comment does not tell much. Please write a proper docstring.

Please delete the comment.

timhoffm · 2026-10-14T15:20:53Z

        self,
        mosaic: str,
        *,
-        sharex: bool = ...,


I'm not clear whether we want "all" as synonym for True. On the one hand it's unnecessarily redundant, on the other hand it's consistent with subplots. I'm slightly inclined towards the narrower API. Others may have stronger opinions.

But if we support "all", we also must "none".

…bels and updated docstrings

nillohitroy · 2026-10-15T11:03:18Z

@timhoffm
I have made the changes as mentioned in the review. Please check if the changes made are correct and again thank you for your valuable feedback.

nillohitroy · 2026-10-21T05:42:43Z

Hi @timhoffm
Could you please check if there are any issues? If not, then could you please approve my PR?

timhoffm · 2026-10-22T13:19:43Z

+        # Testing row sharing for y-axis
+        axd["A"].set_ylim(0, 50)
+        axd["C"].set_ylim(-10, 10)
+
+        assert axd["B"].get_ylim() == (0, 50)
+        assert axd["D"].get_ylim() == (-10, 10)
+        assert axd["E"].get_ylim() != (0, 50)
+
+        axd["A"].set_xlim(-5, 5)
+        axd["C"].set_xlim(100, 200)
+
+        assert axd["E"].get_xlim() == (-5, 5)
+        assert axd["D"].get_xlim() != (100, 200)


While it's internal API, I think it's still clearer to test the actual grouping. Something like (didn't test the actual output order on this PR):

xgroups = [[ax.get_label() for ax in group] for group in Axes._shared_axes['x']] assert xgroups == [["A", "E"], ["B", "F"]] ygroups = [[ax.get_label() for ax in group] for group in Axes._shared_axes['y']] assert ygroups == [["A", "B"], ["C", "D"], ["E", "F"]]

timhoffm · 2026-10-22T13:24:19Z

+
+        msg = "must be True, False, 'all', 'none', 'row', or 'col'"
+        with pytest.raises(ValueError, match=msg):
+            fig.subplot_mosaic(layout, sharex="invalid_string")


Inline the layout as it's not really relevant.

Suggested change

fig.subplot_mosaic(layout, sharex="invalid_string")

fig.subplot_mosaic("AB;CD", sharex="invalid_string")

…est code

Copilot

🟡 Changes recommended

Moderate issues remain with outer-label behavior and the Figure API documentation/release notes.

2 open issues

2 resolved since last review

_{Review effort: Lite}

+                    for ax in group:
+                        # Float tolerance of 1e-5 to account for rounding errors
+                        pos = ax.get_subplotspec().get_position(self)
+                        if pos.ymin > bottom_edge + 1e-5:
+                            ax.tick_params(labelbottom=False)


timhoffm · 2026-10-22T20:27:01Z

+        xgroups = [
+            sorted(ax.get_label() for ax in group)
+            for group in axd["A"]._shared_axes["x"]
+        ]
+        assert ["A", "E"] in xgroups
+        assert ["B", "F"] in xgroups


I want the the full sharing information to be tested, in particular also that nothing else is shared. Let's go with sets instead of sorting to cope with order

Suggested change

xgroups = [

sorted(ax.get_label() for ax in group)

for group in axd["A"]._shared_axes["x"]

]

assert ["A", "E"] in xgroups

assert ["B", "F"] in xgroups

xgroups = {

{ax.get_label() for ax in group}

for group in axd["A"]._shared_axes["x"]

}

assert xgroups ={{"A", "E"}, {"B", "F"}}

also for ygroups.

Since we're going to need this several times, it makes sense to create a helper shared_groups(axis_name) so that tests reduce to one-liners:

assert shared_groups('x') == {{"A", "E"}, {"B", "F"}}

timhoffm · 2026-10-22T20:35:14Z

+    def test_share_row_col_nested(self):
+        layout = [
+            ["A", [["B", "C"],
+                   ["D", "E"]]]
+        ]
+        fig = plt.figure()
+        axd = fig.subplot_mosaic(layout, sharey="row", sharex="col")


What exactly is this testing that may depend on nested?

I'd rather see some non-trivial layouts tested like.

AAB CDD EEF

Please also use the above mentioned group checks rather than checking (non)updated limits.

timhoffm · 2026-10-22T20:45:23Z

+        #         ax.sharey(ax0)
+        #         ax._label_outer_yaxis(skip_non_rectangular_axes=True)
+
+        # Symmetric Sharing


Please delete the comment.

timhoffm · 2026-10-22T20:46:13Z

        ax_dict["A"].set(xscale="log", yscale="logit")
-        assert all(ax.get_xscale() == "log" and ax.get_yscale() == "logit"
-                   for ax in ax_dict.values())
+
+        # If sharing is True or "all", all axes should match "A"
+        if share_val in [True, "all"]:
+            assert all(ax.get_xscale() == "log" and ax.get_yscale() == "logit"
+                       for ax in ax_dict.values())
+        # If sharing is False or "none", only "A" should be changed
+        else:
+            assert ax_dict["A"].get_xscale() == "log"
+            assert ax_dict["B"].get_xscale() != "log"


Please rewrite this to group checks as well. That's much cleaner than the original scale checks.

…ormation

Copilot AI balanced review requested due to automatic review settings October 4, 2026 14:48

Copilot AI reviewed Oct 4, 2026

github-actions Bot added topic: pyplot API topic: figures and subfigures first-contribution labels Oct 4, 2026

github-actions Bot added topic: geometry manager LayoutEngine, Constrained layout, Tight layout topic: rcparams Documentation: devdocs files in doc/devel labels Oct 7, 2026

nillohitroy-ai26 added 2 commits October 8, 2026 06:33

ENH: added sharex/sharey string support to subplot_mosaic

92b8809

Fixed linting errors and updated type hints for issue 18305

1416ea3

nillohitroy force-pushed the subplot_mosaic-share-issue-18305 branch from 4f25789 to 1416ea3 Compare October 8, 2026 01:04

github-actions Bot removed topic: geometry manager LayoutEngine, Constrained layout, Tight layout topic: rcparams Documentation: devdocs files in doc/devel labels Oct 8, 2026

nillohitroy requested a balanced review from Copilot October 12, 2026 15:32

Copilot AI reviewed Oct 12, 2026

nillohitroy requested a lite review from Copilot October 12, 2026 15:32

Copilot started reviewing on behalf of nillohitroy October 12, 2026 15:33

View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

timhoffm reviewed Oct 14, 2026

View reviewed changes

Addressed Review: add 'none' support, use physical coordinates for la…

b9a6ff6

…bels and updated docstrings

timhoffm reviewed Oct 22, 2026

View reviewed changes

Changed function description making it more concise and changed the t…

1186f80

…est code

nillohitroy requested a lite review from Copilot October 22, 2026 17:41

Copilot started reviewing on behalf of nillohitroy October 22, 2026 17:41

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread lib/matplotlib/figure.py

Comment on lines +2393 to +2397

for ax in group:

# Float tolerance of 1e-5 to account for rounding errors

pos = ax.get_subplotspec().get_position(self)

if pos.ymin > bottom_edge + 1e-5:

ax.tick_params(labelbottom=False)

timhoffm reviewed Oct 22, 2026

View reviewed changes

Modified tests to be less verbose and to test on the full sharing inf…

7ef9062

…ormation

	fig.subplot_mosaic(layout, sharex="invalid_string")
	fig.subplot_mosaic("AB;CD", sharex="invalid_string")

Conversation

nillohitroy commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

AI Disclosure

Verification

PR quality check

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 4, 2026

Uh oh!

nillohitroy commented Oct 6, 2026

Uh oh!

iccir commented Oct 6, 2026

Uh oh!

nillohitroy commented Oct 7, 2026

Uh oh!

nillohitroy commented Oct 7, 2026

Uh oh!

rcomer commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iccir commented Oct 7, 2026

Uh oh!

nillohitroy commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iccir commented Oct 8, 2026

Uh oh!

nillohitroy commented Oct 8, 2026

Uh oh!

nillohitroy commented Oct 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

timhoffm Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rcomer commented Oct 13, 2026

Uh oh!

nillohitroy commented Oct 13, 2026

Uh oh!

nillohitroy commented Oct 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nillohitroy commented Oct 15, 2026

Uh oh!

nillohitroy commented Oct 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

nillohitroy commented Oct 4, 2026 •

edited

Loading

rcomer commented Oct 7, 2026 •

edited

Loading

nillohitroy commented Oct 8, 2026 •

edited

Loading

timhoffm Oct 22, 2026 •

edited

Loading