Repository navigation
[Feature add] Share Axes in GeoPlot + bug fixes - #159
cvanelteren merged 156 commits into
Conversation
Sorry, something went wrong.
|
Note: Tests will fail on added tests. |
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 95.36153% with 34 lines in your changes missing coverage. Please review. 📢 Thoughts on this report? Let us know! |
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a feature to share axes on geographic plots and includes bug fixes related to axis sharing and test configuration.
- Adds tests for validating shared geographic tick configurations.
- Refactors axis sharing logic in GeoAxes and related modules.
- Removes extraneous debug prints in test configuration.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file| File | Description |
|---|---|
| ultraplot/tests/test_geographic.py | Adds new tests for geographic ticks and shared axis behavior |
| ultraplot/tests/conftest.py | Removes debug print statements from test failure handling |
| ultraplot/figure.py | Updates subplot creation and axis sharing logic for GeoAxes |
| ultraplot/axes/shared.py | Adjusts share function signatures and minor ticker handling |
| ultraplot/axes/geo.py | Extensive refactor for GeoAxes: sharing, toggling ticks and gridliner labels |
| ultraplot/axes/base.py | Adds an _unshare method and a guard for NaN padding values |
ultraplot/axes/geo.py:1780
- [nitpick] Consider renaming the loop variable 'object' to something like 'obj' to avoid shadowing the built-in 'object'.
for object in objects:
Sorry, something went wrong.
|
Will add more tests tomorrow for codecov but it's looking good locally with a few plots showing different sizes but otherwise have high visual fidelity. Opening this up for review. |
Sorry, something went wrong.
beckermr
left a comment
There was a problem hiding this comment.
I took a closer look at the image test failure:
Here is the code:
@pytest.mark.mpl_image_compare
def test_geoticks():
lonlim = (-140, 60)
latlim = (-10, 50)
basemap_projection = uplt.Proj(
"cyl",
lonlim=lonlim,
latlim=latlim,
backend="basemap",
)
fig, ax = uplt.subplots(
ncols=3,
proj=(
"cyl", # cartopy
"cyl", # cartopy
basemap_projection, # basemap
),
share=0,
)
settings = dict(land=True, labels=True, lonlines=20, latlines=20)
# Shows sensible "default"; uses cartopy backend to show the grid lines with ticks
ax[0].format(
lonlim=lonlim,
latlim=latlim,
**settings,
)
# Add lateral ticks only
ax[1].format(
latticklen=True,
gridminor=True,
lonlim=lonlim,
latlim=latlim,
**settings,
)
ax[2].format(
latticklen=5.0,
lonticklen=2.0,
grid=False,
gridminor=False,
**settings,
)
return fig
Here is a zoom of the middle panel (top is baseline, bottom is this PR):
It appears that the call to set latlim for the middle panel is now being ignored in the new version of the code in this PR. This is definitely a bug. I don't see why latlim should now be ignored.
Sorry, something went wrong.
|
Not sure why the ticks are not using the full range but I cut off the edges here and it looks the same. |
Sorry, something went wrong.
|
weirdly enough when I do this for the basemap the yticks behave strangely. Only one appears. |
Sorry, something went wrong.
beckermr
left a comment
There was a problem hiding this comment.
Two more items. Thank you so much for the good work on this PR!
Sorry, something went wrong.
beckermr
left a comment
There was a problem hiding this comment.
LGTM! Great work on this PR!
Sorry, something went wrong.
|
Will draft an appropriate release to 1.5. Lots of new features 😄 |
Sorry, something went wrong.
- Added ability to share GeoAxes - Fixed numerous bugs regarding ticks on GeoAxes - Backend allows for sharing and unsharing axes. --------- Co-authored-by: Matthew R. Becker <beckermr@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Added ability to share GeoAxes - Fixed numerous bugs regarding ticks on GeoAxes - Backend allows for sharing and unsharing axes. --------- Co-authored-by: Matthew R. Becker <beckermr@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Addresses #155 and #158