Skip to content

Conversation

cvanelteren
Copy link
Collaborator

@cvanelteren cvanelteren commented Sep 16, 2025

Closes #348

This PR refactors axis sharing logic and adds comprehensive tests to improve coverage and robustness across UltraPlot’s core plotting modules.

Key Changes

Core Logic

  • Refactored axis sharing checks in GeoAxes and Figure:

    • Replaced usage of _get_sharing_level() with direct checks on _sharex and _sharey for clarity and correctness.
    • Updated axis sharing logic to use the correct sharing level for each axis (x or y), ensuring more precise control over label and tick sharing.
    • Improved handling of axis sharing for geographic axes, passing the which argument to clarify axis context.
  • Removed unused method: _get_sharing_level() from Figure.

Tests

  • Added new tests in test_figure.py:

    • Coverage for _get_renderer to ensure renderer acquisition works as expected.
    • Coverage for _share_labels_with_others, including both early return (no sharing) and execution with sharing enabled.
  • Added new tests in test_sharing.py:

    • Parametrized tests for x and y axis sharing levels, verifying correct label and tick visibility for all sharing modes and Matplotlib versions.
    • Ensures border axes are correctly detected and labels/ticks are only visible where appropriate.
  • Improved tests in test_geographic.py:

    • Parametrized sharing level tests for geographic axes, ensuring correct sharing of limits and labels.

Why?

  • Improved clarity and maintainability: Directly checking _sharex and _sharey makes the code easier to understand and less error-prone.
  • Better test coverage: Addresses previously uncovered lines and branches, as highlighted by Codecov.
  • More robust axis sharing: Ensures correct behavior for all sharing levels, including edge cases and version differences in Matplotlib.

@cvanelteren cvanelteren changed the title Fix sharing Restore sharing behavior for level 1 Sep 16, 2025
@cvanelteren cvanelteren marked this pull request as draft September 16, 2025 06:51
Copy link

codecov bot commented Sep 16, 2025

@cvanelteren cvanelteren changed the title Restore sharing behavior for level 1 Rm _get_sharing_level and add additional unittests for sharing Sep 16, 2025
@cvanelteren cvanelteren changed the title Rm _get_sharing_level and add additional unittests for sharing Refactor axis sharing logic and improve test coverage for UltraPlot Sep 16, 2025
@cvanelteren cvanelteren marked this pull request as ready for review September 19, 2025 10:55
@cvanelteren cvanelteren requested a review from Copilot September 19, 2025 10:58
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors axis sharing logic in UltraPlot by replacing a centralized sharing level method with direct checks on sharing attributes, and adds comprehensive test coverage for axis sharing behavior across different modules.

  • Removes the _get_sharing_level() method and replaces its usage with direct checks on _sharex and _sharey attributes
  • Adds parametrized tests for axis sharing levels in both regular and geographic plotting contexts
  • Improves test coverage for renderer acquisition and label sharing methods

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
ultraplot/tests/test_sharing.py New comprehensive test file covering x/y axis sharing levels with parametrized tests
ultraplot/tests/test_geographic.py Converts sharing level test to parametrized format and updates sharing logic reference
ultraplot/tests/test_figure.py Adds tests for renderer acquisition and label sharing methods
ultraplot/figure.py Removes _get_sharing_level() method and updates sharing checks to use direct attribute access
ultraplot/axes/geo.py Updates axis sharing logic to use direct sharing level checks and adds which parameter

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

cvanelteren and others added 2 commits September 19, 2025 13:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cvanelteren cvanelteren marked this pull request as ready for review October 17, 2025 15:43
@cvanelteren cvanelteren requested a review from beckermr October 17, 2025 15:43
@cvanelteren
Copy link
Collaborator Author

cvanelteren commented Oct 17, 2025

Some tests fail due to image mismatch but visually look ok; sometimes there is a title that is slightly shifted. The test test_geo_with_panels now fails because I copied the formatters. Panels for geo now copy the formatters for the relevant range (y or x) so that plotting is within the same data range. Formatters are updated too when relevant. This effectively creates some crowding on the x-values in test_geo_with_panels

@beckermr
Copy link
Collaborator

This PR is pretty hefty and so will take time for me to review. Thanks for the hard work!

@cvanelteren
Copy link
Collaborator Author

Thanks. Thinking of breaking it up to make it more manageable. Will see if I can do that to ensure tractability.

@cvanelteren
Copy link
Collaborator Author

Will do a diff tomorrow!

@cvanelteren
Copy link
Collaborator Author

I split this PR into two, see #372 and #373. The should be handled in sequential order @beckermr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent behavior of axis label sharing

2 participants