-
Notifications
You must be signed in to change notification settings - Fork 18
Fix GeoAxes panel alignment with aspect-constrained projections #432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
cvanelteren
wants to merge
20
commits into
main
Choose a base branch
from
fix/geoaxes-panel-alignment
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
fa1e548
Fix GeoAxes panel alignment with aspect-constrained projections
cvanelteren ef55f69
Fix double-adjustment issue in panel positioning
cvanelteren 92b0987
Merge branch 'main' into fix/geoaxes-panel-alignment
cvanelteren 83f33fd
Revert "Fix double-adjustment issue in panel positioning"
cvanelteren 85b5c75
Fix panel gap calculation to use original positions
cvanelteren fc870f3
Adjust tolerance in test_reference_aspect for floating-point precision
cvanelteren 794e7a5
Fix boundary label visibility issue in cartopy
cvanelteren b9487bd
Revert "Fix boundary label visibility issue in cartopy"
cvanelteren 57012d7
Fix test_boundary_labels tests to match actual cartopy behavior
cvanelteren add1e2b
Remove _adjust_panel_positions call from GeoAxes.draw()
cvanelteren 5dd4f92
Override draw() in _CartopyAxes to adjust panel positions
cvanelteren 800f983
make subplots_adjust work with both backend
cvanelteren b4898ba
Revert "make subplots_adjust work with both backend"
cvanelteren ae23823
this works but generates different sizes
cvanelteren 85ed02c
fix failing tests
cvanelteren 5bba3eb
this fails locally but should pass on GHA
cvanelteren e935127
Merge branch 'main' into fix/geoaxes-panel-alignment
cvanelteren f629693
Fix unequal slicing for Gridspec (#435)
cvanelteren 13db730
fix remaining issues
cvanelteren 3a1b6fb
Merge branch 'main' into fix/geoaxes-panel-alignment
cvanelteren File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1473,26 +1473,26 @@ def _check_boundary_labels(ax, expected_lon_labels, expected_lat_labels): | |
| # Check xlim/ylim are expanded beyond actual limits | ||
| assert hasattr(gl, "xlim") and hasattr(gl, "ylim") | ||
|
|
||
| # Check longitude labels | ||
| # Check longitude labels - only verify the visible ones match expected | ||
| lon_texts = [ | ||
| label.get_text() for label in gl.bottom_label_artists if label.get_visible() | ||
| ] | ||
| assert len(gl.bottom_label_artists) == len(expected_lon_labels), ( | ||
| f"Should have {len(expected_lon_labels)} longitude labels, " | ||
| f"got {len(gl.bottom_label_artists)}" | ||
| assert len(lon_texts) == len(expected_lon_labels), ( | ||
| f"Should have {len(expected_lon_labels)} visible longitude labels, " | ||
| f"got {len(lon_texts)}: {lon_texts}" | ||
| ) | ||
| for expected in expected_lon_labels: | ||
| assert any( | ||
| expected in text for text in lon_texts | ||
| ), f"{expected} label should be visible, got: {lon_texts}" | ||
|
|
||
| # Check latitude labels | ||
| # Check latitude labels - only verify the visible ones match expected | ||
| lat_texts = [ | ||
| label.get_text() for label in gl.left_label_artists if label.get_visible() | ||
| ] | ||
| assert len(gl.left_label_artists) == len(expected_lat_labels), ( | ||
| f"Should have {len(expected_lat_labels)} latitude labels, " | ||
| f"got {len(gl.left_label_artists)}" | ||
| assert len(lat_texts) == len(expected_lat_labels), ( | ||
| f"Should have {len(expected_lat_labels)} visible latitude labels, " | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here can be ignored |
||
| f"got {len(lat_texts)}: {lat_texts}" | ||
| ) | ||
| for expected in expected_lat_labels: | ||
| assert any( | ||
|
|
@@ -1535,7 +1535,13 @@ def test_boundary_labels_negative_longitude(): | |
| grid=False, | ||
| ) | ||
| fig.canvas.draw() | ||
| _check_boundary_labels(ax[0], ["120°W", "90°W", "60°W"], ["20°N", "35°N", "50°N"]) | ||
| # Note: Cartopy hides the boundary label at 20°N due to it being exactly at the limit | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. editor changed this -- can ignore |
||
| # This is expected cartopy behavior with floating point precision at boundaries | ||
| _check_boundary_labels( | ||
| ax[0], | ||
| ["120°W", "90°W", "60°W"], | ||
| ["20°N", "35°N", "50°N"], | ||
| ) | ||
| uplt.close(fig) | ||
|
|
||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments below