Fix silent lon wrapping for projected (non-spherical) UGRID grids#1525
Open
rajeeja wants to merge 2 commits into
Open
Fix silent lon wrapping for projected (non-spherical) UGRID grids#1525rajeeja wants to merge 2 commits into
rajeeja wants to merge 2 commits into
Conversation
Fixes #1524. _is_projected_grid() in coordinates.py detects projected coordinates from CF standard_name ("projection_x_coordinate"), length units (m, km, ...), or a non-latlon grid_mapping variable — in that priority order, falling back to geographic (backward-compatible). _set_desired_longitude_range skips wrapping and emits a UserWarning when projected coordinates are detected, naming which spherical operations are invalid. Coordinates are preserved as-is. 4 tests: no-wrap on load, units detection, grid_mapping detection, geographic wrapping still works.
rajeeja
force-pushed
the
feat/projected-grid-support
branch
from
fa4d70b to
52e767e
Compare
rajeeja
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where UXarray would silently wrap node_lon into [-180, 180] whenever values exceeded 180, even when the grid’s coordinates are projected (e.g., meters) rather than geographic degrees, corrupting projected UGRID meshes (Fixes #1524).
Changes:
- Added CF-metadata-based projected-grid detection (
_is_projected_grid) and skip longitude wrapping for projected coordinates with aUserWarning. - Added tests covering projected detection via
standard_name,units, andgrid_mapping, plus a regression test ensuring geographic wrapping still occurs. - Documented projected (non-spherical) UGRID behavior and limitations in the user guide.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
uxarray/grid/coordinates.py |
Adds projected-grid detection and guards longitude wrapping with a warning for projected coordinates. |
test/grid/grid/test_projected.py |
Adds regression/unit tests for projected detection and safe loading behavior. |
docs/user-guide/grid-formats.rst |
Documents projected-coordinate detection signals, warning behavior, and limitations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+740
to
+754
| if _is_projected_grid(uxgrid): | ||
| import warnings | ||
|
|
||
| warnings.warn( | ||
| "Projected (non-spherical) coordinates detected on this grid " | ||
| "(standard_name='projection_x_coordinate' or length units). " | ||
| "UXarray's geometry algorithms (face areas, GCA intersections, " | ||
| "zonal mean, bounds, cross-sections) assume a unit sphere and " | ||
| "will produce incorrect results on projected grids. " | ||
| "Loading, plotting, and connectivity operations are unaffected. " | ||
| "Check Grid.is_projected for the detected coordinate type.", | ||
| UserWarning, | ||
| stacklevel=3, | ||
| ) | ||
| return |
Comment on lines
+714
to
+716
| units = attrs.get("units", "").lower().strip() | ||
| _ANGULAR_UNITS = {"degrees_east", "degrees", "degree", "deg", "rad", "radians"} | ||
| _LENGTH_UNITS = {"m", "km", "meter", "meters", "metre", "metres", "ft", "feet"} |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1524.
Problem
_set_desired_longitude_rangeapplied(x + 180) % 360 - 180unconditionally whenevernode_lon.max() > 180— no check onunitsorstandard_name. Projected coordinates in meters (e.g. x ~ 1.5e6) were silently corrupted to garbage degree values with no warning.Fix
Add
_is_projected_grid()incoordinates.pythat reads CF metadata already present on the renamednode_lonvariable, in priority order:standard_name = "projection_x_coordinate"→ projectedstandard_name = "longitude"→ geographic (short-circuit)unitsis a length unit (m,km,ft, …) → projectedunitsis angular (degrees_east, …) → geographicgrid_mappingvariable withgrid_mapping_name != "latitude_longitude"→ projected_set_desired_longitude_rangeskips wrapping when projected and emits aUserWarninglisting which spherical operations are invalid (face areas, GCA intersections, zonal mean, bounds, cross-sections). Loading, plotting, and connectivity are unaffected.Notes
is_projected/is_geographicproperties are deferred to a future CRS-design PRfrom_topologylimitation: raw arrays carry no CF attrs, so detection cannot fire; callers must usefrom_datasetwith an attributedxr.DatasetNODE_LON_ATTRSwithstandard_name="longitude"explicitly