Test/update label sharing tests #372

cvanelteren · 2025-10-18T11:23:06Z

The previous implementation had complex and distributed logic for determining tick and label visibility, which was difficult to maintain and extend. This refactoring centralizes the logic within the Figure class, making it more robust and easier to understand.
Summary

Refactored figure-level ticklabel sharing and fixed a side-leak bug
Standardized tick_params key usage across Matplotlib versions via per-axis key mapping
Made panel-group sharing symmetric so top/right-only panels promote sharing level to 3
Kept behavior and warnings for unsupported/mixed subplot types

Files changed

ultraplot/ultraplot/figure.py
- Refactor _share_ticklabels into smaller helpers:
  - New private helpers:
    - _label_key_map(): computes version-dependent label keys for tick_params.
    - _group_axes_by_axis(axes, axis): groups axes by row (x) or column (y).
    - _compute_baseline_tick_state(group_axes, axis, label_keys): builds baseline ticklabel state from main axes only; warns on unsupported/mixed types.
    - _apply_border_mask(axi, baseline, sides, outer_axes): enforces figure borders and suppresses opposite panel sides; uses per-axis _label_key.
    - _effective_share_level(axi, axis, sides): computes effective share level considering panel groups and neighbors; checks both relevant sides (fixes prior “last side wins” leak).
    - _set_ticklabel_state(axi, axis, state): applies state to cartesian via set_tick_params (with x/y-to-bool cleanup) or to geo via _toggle_gridliner_labels.
  - Behavior retained:
    - Baseline built only from main axes.
    - Panels included in application.
    - Warnings for mixed types or unsupported axes.
    - Geo/cartesian pathways preserved.
  - Bug fix:
    - Removed reliance on a loop-local side after iteration; share-level promotion now checks all relevant sides for the given axis.
- Panel creation: use per-axis _label_key mapping when reading/writing tick_params
  - When disabling main-axis tick labels due to a panel:
    - Replaced direct calls like ax.xaxis.set_tick_params(labeltop=False) with ax.xaxis.set_tick_params(**{ax._label_key("labeltop"): False}) (and similarly for labelbottom, labelleft, labelright).
  - When setting panel axis labels:
    - Writes use pax._label_key(...).
    - Reads use ax._label_key(...) for get_tick_params()[...].
  - Cleanup:
    - Removed a stray debug print in the effective share-level logic (seen during edit context).
ultraplot/ultraplot/axes/base.py
- _apply_auto_share():
  - Make panel-group membership symmetric:
    - For x: if there are top panels, set self._panel_sharex_group = True so the main axes is included in the x-panel group even without bottom panels.
    - For y: if there are right panels, set self._panel_sharey_group = True so the main axes is included in the y-panel group even without left panels.
  - Outcome:
    - With only right (or top) panels present, sharing level now correctly promotes to 3 for that axis on the main axes.
Tests
- Numerous tests are added and changed to ensure that we offer a robust verification of the new logic.

Behavioral notes

Matplotlib ≤ 3.9 compatibility: all new writes/reads to tick_params use per-axis _label_key to map labeltop/labelbottom to labelright/labelleft as needed.
Warnings for mixed/unsupported subplot types remain unchanged.
Figure stale semantics preserved.

User-visible impact

Ticks should be shared "as one would expect"; if one can draw a line from one axis without hitting another axis, the ticks should be visible when appropriate, otherwise they are shared.

This commit introduces a major refactoring of the tick and label sharing mechanism in UltraPlot. The previous implementation had complex and distributed logic for determining tick and label visibility, which was difficult to maintain and extend. This refactoring centralizes the logic within the `Figure` class, making it more robust and easier to understand. Key changes: - A new `_share_ticklabels` method in `Figure` now handles all tick label sharing. - The `_get_border_axes` method has been improved to be more accurate. - The `_Crawler` utility in `ultraplot/utils.py` has been rewritten to better handle complex layouts with panels and mixed axes types. - Redundant and complex logic has been removed from `CartesianAxes`, `GeoAxes`, and other modules.

This commit updates the test suite to align with the new tick and label sharing mechanism. Key changes: - Added `fig.canvas.draw()` calls in numerous tests to ensure that the new, deferred label sharing logic is triggered before assertions. - Updated assertions in tests for `CartesianAxes`, `GeoAxes`, and subplots to match the expected behavior of the refactored implementation. - Added new tests to cover more complex scenarios with panels and mixed axes types.

for more information, see https://pre-commit.ci

codecov · 2025-10-18T11:37:40Z

Codecov Report

❌ Patch coverage is 95.27559% with 30 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ultraplot/figure.py	90.64%	10 Missing and 9 partials ⚠️
ultraplot/utils.py	91.66%	2 Missing and 2 partials ⚠️
ultraplot/gridspec.py	87.50%	2 Missing and 1 partial ⚠️
ultraplot/tests/test_axes.py	86.66%	2 Missing ⚠️
ultraplot/axes/polar.py	85.71%	1 Missing ⚠️
ultraplot/tests/test_subplots.py	99.47%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

beckermr

What is the difference between this PR and the other one that is similar?

ultraplot/gridspec.py

beckermr · 2025-10-22T17:49:20Z

The following tests need a closer look:

test_panels_without_sharing_1 (change in tick labels on main plot y-axis)
test_uneven_levels (change in x-axis tick labels)
test_geo_with_panels (change in bottom-right panel tick labels and ticks)
test_align_labels (change in panel E x tick labels)
test_share_all_basic (everything looks broken, x axis range is wrong)

Some of these cases look like bugs. Others might be the desired outcome. I don't know which is which.

cvanelteren · 2025-10-23T07:48:47Z

Thanks for the careful review @beckermr! I think the ticks are related to the axes limits not being shared. Not sure how that changed -- but will get to the bottom of this. The tick params for the axis I fixed locally. Will check the final ones and recheck. After this I will get back to you.

cvanelteren · 2025-10-23T07:51:28Z

What is the difference between this PR and the other one that is similar?

I split it to avoid overwhelming number of changes; #372 and #372 are together the same as #349

cvanelteren · 2025-10-23T13:02:27Z

Weird I set the limits explicitly in the function _add_panel_axes and then the sharing broke. I removed that stuff now and check the generated results.

cvanelteren · 2025-10-23T15:54:46Z

Looking better!

What should we decide on test_geo_with_panels do we set the ticks to be on depending on the main axis? @beckermr

beckermr · 2025-10-23T18:31:55Z

I took a look again. I see the following:

test_panels_without_sharing_1: The panel on the right axis has its own tick labels in the new code. Is that correct?
test_geo_with_panels: The panels do not have tick labels and the case you asked about.

What should we decide on test_geo_with_panels do we set the ticks to be on depending on the main axis?

Yes. I'd say if the main axis is a geo axis, then the panel should have tick labels on since its axis is not shared.

cvanelteren · 2025-10-23T19:11:11Z

For test_panels_without_sharing the right plot should have the ticks where they are now:

    fig, axs = uplt.subplots(ncols=2, share=True, refwidth=1.5, includepanels=False)
    axs.panel("left", share=False)
    fig.format(ylabel="ylabel", xlabel="xlabel")

The logic for the current panel for the geo would need to change if we want that to be off for the test that is currently failing. Right now it will check if it is on a border but will automatically draw it for right axes; I am fine with it as is, as the default right now also puts these ticks on. However, it may make more sense to make it dependent on the main axis in general. Since in this case all ticks are off, it should also be off for the panel; but open to both tbh.

beckermr

There are a few unittest failures, but otherwise, LGTM!

cvanelteren · 2025-10-24T16:18:10Z

Found the issue with the geo labels; auto had to be set when calling _set_lim.

beckermr · 2025-10-24T16:30:27Z

What is this error on MPL 39?

FAILED ultraplot/tests/test_figure.py::test_suptitle_alignment - KeyError: 'labeltop'
FAILED ultraplot/tests/test_geographic.py::test_panels_geo - KeyError: 'labeltop'
FAILED ultraplot/tests/test_inset.py::test_inset_basic - KeyError: 'labeltop'
FAILED ultraplot/tests/test_statistical_plotting.py::test_panel_dist - KeyError: 'labeltop'
FAILED ultraplot/tests/test_subplots.py::test_title_deflection - KeyError: 'labeltop'
FAILED ultraplot/tests/test_subplots.py::test_panel_sharing_top_right[layout0] - KeyError: 'labeltop'
FAILED ultraplot/tests/test_subplots.py::test_panel_ticklabels_all_sides_share_and_no_share[True] - KeyError: 'labeltop'

cvanelteren · 2025-10-24T17:10:21Z

The key labelling changed in mpl 3.10; I thought I provided a fix for this with a translation function. Will carefully examine. Still ironing out some odd edge cases

cvanelteren · 2025-10-24T18:16:31Z

Will add some more tests tomorrow. Every combination is passing on the tests now indicating that the labels are correctly used internally. May also change how the private conversion handles directly by making it available as a global var. A bit annoying to make this dual compatibility.

cvanelteren · 2025-10-25T08:42:31Z

I think I added sufficient number of tests now to make this merge. I will wait for the tests to finish. I updated the text on top for internal reference so we can track what changed exactly. As most of the stuff is in wards facing it is key to keep track of what we changed so we don't get lost -- as I did many times in this PR.

I think the sharing is more robust for the ticks now and allows for flexible sharing. I will turn to #373 as that one follows this PR and probable needs updating.

cvanelteren · 2025-10-25T09:04:14Z

Failing tests are expected. Merging.

cvanelteren added 2 commits October 18, 2025 13:12

cvanelteren mentioned this pull request Oct 18, 2025

Refactor axis sharing logic and improve test coverage for UltraPlot #349

Closed

cvanelteren added the pre-commit.ci autofix label Oct 18, 2025

pre-commit-ci bot removed the pre-commit.ci autofix label Oct 18, 2025

[pre-commit.ci] auto fixes from pre-commit.com hooks

f75e987

for more information, see https://pre-commit.ci

cvanelteren added the pre-commit.ci autofix label Oct 18, 2025

pre-commit-ci bot removed the pre-commit.ci autofix label Oct 18, 2025

cvanelteren mentioned this pull request Oct 22, 2025

Allow non-rectangular grids to use side labels #376

Merged

beckermr requested changes Oct 22, 2025

View reviewed changes

beckermr reviewed Oct 22, 2025

View reviewed changes

ultraplot/gridspec.py Outdated Show resolved Hide resolved

fix edge cases

b8c7155

cvanelteren added 4 commits October 23, 2025 11:30

stash

9180c93

restore gridspec hidden parameter defaults

4ba0b5b

revert explicit axis limit setting

e688c37

Merge branch 'main' into test/update-label-sharing-tests

0b48a78

cvanelteren added 5 commits October 23, 2025 15:09

bump merge

e23064f

use new grid

c6f496c

rm ticks for twin

219aab6

add scale override

7488032

bump

06646c5

cvanelteren requested a review from beckermr October 23, 2025 15:54

beckermr approved these changes Oct 24, 2025

View reviewed changes

default lim to auto

6c2bbd2

cvanelteren added 6 commits October 24, 2025 19:26

make mpl compatible and refactor into smaller functions

ebeba47

skip colorbars on crawl

c1691dd

forgot this chunk

bde7626

restore test and remove debug

cf4e2ac

make panel sharing symmetric

759926b

add additional guards

73eb1e6

cvanelteren added 12 commits October 25, 2025 09:58

more tests to check if guards are working

1461159

guard against non-rectilinear shares

80374b8

Ensure that colorbars and legend do no interfere with layout

deb2a31

Check that colorbars do not interfere with layout

5f20dea

more stress tests

3ae2a3a

bump tests

25f7d63

fix spelling

c857e80

bump tests

cd383b4

bump test

b93f41f

bump test

4d60108

remove show

25a6cb8

add simple share test

8152429

cvanelteren enabled auto-merge (squash) October 25, 2025 09:04

cvanelteren disabled auto-merge October 25, 2025 09:04

cvanelteren merged commit 0a121c9 into Ultraplot:main Oct 25, 2025
16 of 25 checks passed

cvanelteren mentioned this pull request Oct 25, 2025

Refactor: Improve tick and label sharing logic #373

Closed

Test/update label sharing tests #372

Test/update label sharing tests #372

Uh oh!

Conversation

cvanelteren commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

beckermr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

beckermr commented Oct 22, 2025

Uh oh!

cvanelteren commented Oct 23, 2025

Uh oh!

cvanelteren commented Oct 23, 2025

Uh oh!

cvanelteren commented Oct 23, 2025

Uh oh!

cvanelteren commented Oct 23, 2025

Uh oh!

beckermr commented Oct 23, 2025

Uh oh!

cvanelteren commented Oct 23, 2025

Uh oh!

beckermr left a comment

Choose a reason for hiding this comment

Uh oh!

cvanelteren commented Oct 24, 2025

Uh oh!

beckermr commented Oct 24, 2025

Uh oh!

cvanelteren commented Oct 24, 2025

Uh oh!

cvanelteren commented Oct 24, 2025

Uh oh!

cvanelteren commented Oct 25, 2025

Uh oh!

cvanelteren commented Oct 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cvanelteren commented Oct 18, 2025 •

edited

Loading

codecov bot commented Oct 18, 2025 •

edited

Loading