From 6c9c1047074da3ce744327f7a6208a0270747d73 Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:48:48 -0400 Subject: [PATCH 1/7] tested v5 of cleaning stray data Time was getting into grid data, causing issues when subsetting down the line. This should clean the data in a way that shouldn't happen in the future --- uxarray/io/_ugrid.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/uxarray/io/_ugrid.py b/uxarray/io/_ugrid.py index a66dc8c18..4d17eb5bc 100644 --- a/uxarray/io/_ugrid.py +++ b/uxarray/io/_ugrid.py @@ -4,7 +4,7 @@ import uxarray.conventions.ugrid as ugrid from uxarray.constants import INT_DTYPE, INT_FILL_VALUE from uxarray.grid.connectivity import _replace_fill_values - +from uxarray.conventions.descriptors import DESCRIPTOR_NAMES def _read_ugrid(ds): """Parses an unstructured grid dataset and encodes it in the UGRID @@ -82,8 +82,30 @@ def _read_ugrid(ds): ds = ds.swap_dims(dim_dict) + # Strip non-grid extras (e.g. a stray scalar `time` coordinate, or unrelated data variables + ds = _keep_only_grid_vars(ds) + return ds, dim_dict +def _keep_only_grid_vars(ds): + """Return ``ds`` with only recognized UGRID grid variables/coordinates. + + Anything else on the dataset (a stray scalar ``time`` coordinate, unrelated + data variables, etc.) is dropped so it cannot leak onto ``grid._ds``. + + Runs on the file-read path only + """ + # uxarray's own canonical grid-variable names (the same lists Grid filters against) + keep = {"grid_topology"} + keep.update(ugrid.SPHERICAL_COORD_NAMES) # node/edge/face lon-lat + keep.update(ugrid.CARTESIAN_COORD_NAMES) # node/edge/face x-y-z + keep.update(ugrid.CONNECTIVITY_NAMES) # face_node_connectivity, edge_node_connectivity, ... + keep.update(DESCRIPTOR_NAMES) # n_nodes_per_face, face_areas, boundary_*_indices, ... + + # drop_vars removes variables/coords by name (never bare dimensions), so grid + # dims survive with their variables; errors="ignore" tolerates absent names. + drop = [name for name in ds.variables if name not in keep] + return ds.drop_vars(drop, errors="ignore") def _encode_ugrid(ds): """Encodes an unstructured grid represented under a ``Grid`` object as a From 621b0721696e531cc88d4c6446257a239fa23819 Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:52:28 -0400 Subject: [PATCH 2/7] reordered imports for pre-commit.ci --- uxarray/io/_ugrid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uxarray/io/_ugrid.py b/uxarray/io/_ugrid.py index 4d17eb5bc..a1ddca448 100644 --- a/uxarray/io/_ugrid.py +++ b/uxarray/io/_ugrid.py @@ -3,8 +3,8 @@ import uxarray.conventions.ugrid as ugrid from uxarray.constants import INT_DTYPE, INT_FILL_VALUE -from uxarray.grid.connectivity import _replace_fill_values from uxarray.conventions.descriptors import DESCRIPTOR_NAMES +from uxarray.grid.connectivity import _replace_fill_values def _read_ugrid(ds): """Parses an unstructured grid dataset and encodes it in the UGRID From 36031ec593a7e3a1313019b85877ad23fe0cf60f Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:43:09 -0400 Subject: [PATCH 3/7] ran `pre-commit run ...` pre-commit run --files uxarray/io/_ugrid.py Seemed to turn up a few issues with spaces and line lengths --- uxarray/io/_ugrid.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/uxarray/io/_ugrid.py b/uxarray/io/_ugrid.py index a1ddca448..91d4b7e6a 100644 --- a/uxarray/io/_ugrid.py +++ b/uxarray/io/_ugrid.py @@ -6,6 +6,7 @@ from uxarray.conventions.descriptors import DESCRIPTOR_NAMES from uxarray.grid.connectivity import _replace_fill_values + def _read_ugrid(ds): """Parses an unstructured grid dataset and encodes it in the UGRID conventions.""" @@ -87,6 +88,7 @@ def _read_ugrid(ds): return ds, dim_dict + def _keep_only_grid_vars(ds): """Return ``ds`` with only recognized UGRID grid variables/coordinates. @@ -97,16 +99,21 @@ def _keep_only_grid_vars(ds): """ # uxarray's own canonical grid-variable names (the same lists Grid filters against) keep = {"grid_topology"} - keep.update(ugrid.SPHERICAL_COORD_NAMES) # node/edge/face lon-lat - keep.update(ugrid.CARTESIAN_COORD_NAMES) # node/edge/face x-y-z - keep.update(ugrid.CONNECTIVITY_NAMES) # face_node_connectivity, edge_node_connectivity, ... - keep.update(DESCRIPTOR_NAMES) # n_nodes_per_face, face_areas, boundary_*_indices, ... + keep.update(ugrid.SPHERICAL_COORD_NAMES) # node/edge/face lon-lat + keep.update(ugrid.CARTESIAN_COORD_NAMES) # node/edge/face x-y-z + keep.update( + ugrid.CONNECTIVITY_NAMES + ) # face_node_connectivity, edge_node_connectivity, ... + keep.update( + DESCRIPTOR_NAMES + ) # n_nodes_per_face, face_areas, boundary_*_indices, ... # drop_vars removes variables/coords by name (never bare dimensions), so grid # dims survive with their variables; errors="ignore" tolerates absent names. drop = [name for name in ds.variables if name not in keep] return ds.drop_vars(drop, errors="ignore") + def _encode_ugrid(ds): """Encodes an unstructured grid represented under a ``Grid`` object as a ``xr.Dataset`` with an updated grid topology variable.""" From 6ad226b285e920646ab250d5aa3e42c5a1dd2506 Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:42:09 -0400 Subject: [PATCH 4/7] rewrite for fix in grid.py --- uxarray/grid/grid.py | 18 ++++++++++++++++-- uxarray/io/_ugrid.py | 29 ----------------------------- 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/uxarray/grid/grid.py b/uxarray/grid/grid.py index 3c86890f2..2638d0382 100644 --- a/uxarray/grid/grid.py +++ b/uxarray/grid/grid.py @@ -107,6 +107,18 @@ from uxarray.core.dataarray import UxDataArray +def _drop_non_grid_coords(ds): + """Drop coordinates that aren't recognized grid coordinates (e.g. a stray ``time`` + carried in from the source file). + + Coordinate-only, so grid data variables — connectivity, descriptors, and the + subset's ``subgrid_*_indices`` — are always left intact. + """ + grid_coords = set(ugrid.SPHERICAL_COORD_NAMES) | set(ugrid.CARTESIAN_COORD_NAMES) + stray = [coord for coord in ds.coords if coord not in grid_coords] + return ds.drop_vars(stray, errors="ignore") + + class Grid: """Represents a two-dimensional unstructured grid encoded following the UGRID conventions and provides grid-specific functionality. @@ -190,8 +202,10 @@ def __init__( # source grid specification (i.e. UGRID, MPAS, SCRIP, etc.) self.source_grid_spec = source_grid_spec - # internal xarray dataset for storing grid variables - self._ds = grid_ds + # internal xarray dataset for storing grid variables. + # drop stray coordinates (e.g. a `time` carried in from the source file) so they + # can't leak onto the grid and collide during subsetting (see #1444). + self._ds = _drop_non_grid_coords(grid_ds) # source grid specification (i.e. UGRID, MPAS, SCRIP, etc.) self.source_grid_spec = source_grid_spec diff --git a/uxarray/io/_ugrid.py b/uxarray/io/_ugrid.py index 91d4b7e6a..a66dc8c18 100644 --- a/uxarray/io/_ugrid.py +++ b/uxarray/io/_ugrid.py @@ -3,7 +3,6 @@ import uxarray.conventions.ugrid as ugrid from uxarray.constants import INT_DTYPE, INT_FILL_VALUE -from uxarray.conventions.descriptors import DESCRIPTOR_NAMES from uxarray.grid.connectivity import _replace_fill_values @@ -83,37 +82,9 @@ def _read_ugrid(ds): ds = ds.swap_dims(dim_dict) - # Strip non-grid extras (e.g. a stray scalar `time` coordinate, or unrelated data variables - ds = _keep_only_grid_vars(ds) - return ds, dim_dict -def _keep_only_grid_vars(ds): - """Return ``ds`` with only recognized UGRID grid variables/coordinates. - - Anything else on the dataset (a stray scalar ``time`` coordinate, unrelated - data variables, etc.) is dropped so it cannot leak onto ``grid._ds``. - - Runs on the file-read path only - """ - # uxarray's own canonical grid-variable names (the same lists Grid filters against) - keep = {"grid_topology"} - keep.update(ugrid.SPHERICAL_COORD_NAMES) # node/edge/face lon-lat - keep.update(ugrid.CARTESIAN_COORD_NAMES) # node/edge/face x-y-z - keep.update( - ugrid.CONNECTIVITY_NAMES - ) # face_node_connectivity, edge_node_connectivity, ... - keep.update( - DESCRIPTOR_NAMES - ) # n_nodes_per_face, face_areas, boundary_*_indices, ... - - # drop_vars removes variables/coords by name (never bare dimensions), so grid - # dims survive with their variables; errors="ignore" tolerates absent names. - drop = [name for name in ds.variables if name not in keep] - return ds.drop_vars(drop, errors="ignore") - - def _encode_ugrid(ds): """Encodes an unstructured grid represented under a ``Grid`` object as a ``xr.Dataset`` with an updated grid topology variable.""" From eb2960f28c5f229a080c79483910c12f58bc0de6 Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:25:41 -0400 Subject: [PATCH 5/7] added test for new "_drop_non_grid_coords" function --- test/test_subset.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/test_subset.py b/test/test_subset.py index 5f039bf13..88a4b4ff3 100644 --- a/test/test_subset.py +++ b/test/test_subset.py @@ -1,6 +1,8 @@ import uxarray as ux +import numpy as np import pytest +import xarray as xr def test_repr(gridpath, datasetpath): @@ -207,3 +209,31 @@ def test_empty_subset(gridpath, datasetpath): assert res.size == 0 # should still have all the same dim names even if resulting subset is empty: assert set(res.dims) == set(arr.dims) + + +def test_bounding_box_with_stray_grid_time(gridpath): + """Subsetting must work when the grid was built from a source that carried a stray + scalar ``time`` coordinate. Ensures issue #1444 has been fixed. + """ + # grid built from a source carrying a stray scalar `time` + grid_ds = xr.open_dataset(gridpath("ugrid", "quad-hexagon", "grid.nc")) + uxgrid = ux.open_grid(grid_ds.assign_coords(time=np.datetime64("2016-01-01"))) + + # the stray coordinate is dropped during grid construction + assert "time" not in uxgrid._ds.coords + + # a face-centered variable carrying a *different* time coordinate + times = np.array(["2018-01-01", "2020-01-01"], dtype="datetime64[ns]") + uxda = ux.UxDataArray( + data=np.ones((times.size, uxgrid.n_face)), + dims=("time", "n_face"), + coords={"time": times}, + uxgrid=uxgrid, + ) + + # previously raised: "IndexError: dimension coordinate 'time' conflicts ..." + res = uxda.subset.bounding_box(lon_bounds=(-10, 10), lat_bounds=(-10, 10)) + + assert isinstance(res, ux.UxDataArray) + assert res.sizes["time"] == times.size + assert "n_face" in res.dims From 30880fbbd069a243be7655ea296493ec0a5208d9 Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:07:02 -0400 Subject: [PATCH 6/7] stray coordinates on grid files change --- test/test_subset.py | 33 ++++++++++++++++++++++++++++++++- uxarray/grid/grid.py | 14 +------------- uxarray/grid/utils.py | 13 +++++++++++++ 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/test/test_subset.py b/test/test_subset.py index 87a63bd75..ad4cf633b 100644 --- a/test/test_subset.py +++ b/test/test_subset.py @@ -4,7 +4,6 @@ import uxarray.grid.slice as slice_module from uxarray.grid.slice import _remap_dense, _remap_kernel, _remap_searchsorted -import numpy as np import pytest import xarray as xr @@ -361,3 +360,35 @@ def test_bounding_box_with_stray_grid_time(gridpath): assert isinstance(res, ux.UxDataArray) assert res.sizes["time"] == times.size assert "n_face" in res.dims + + +def test_bounding_box_with_stray_grid_time_dimension(gridpath): + """Subsetting must also work when the grid source carried a full ``time`` *dimension* + (a variable along ``time``) rather than the scalar coordinate of issue #1444. + + A dimensional coordinate has dims ``(time,)``, which is not a subset of the + ``(n_face,)`` ``subgrid_face_indices`` indexer, so — unlike a 0-d coordinate — it + never attaches to that indexer and cannot collide during ``.isel``. The stray + dimension harmlessly rides along in ``Grid._ds`` but never reaches the indexing path. + """ + # grid built from a source carrying a variable along a stray `time` dimension + grid_ds = xr.open_dataset(gridpath("ugrid", "quad-hexagon", "grid.nc")) + uxgrid = ux.open_grid(grid_ds.assign(stray=("time", [0.0, 1.0]))) + + # the bare dimension is not a coordinate, so it is not dropped; that is harmless + assert "time" in uxgrid._ds.dims + + # a face-centered variable carrying its own time coordinate + times = np.array(["2018-01-01", "2020-01-01"], dtype="datetime64[ns]") + uxda = ux.UxDataArray( + data=np.ones((times.size, uxgrid.n_face)), + dims=("time", "n_face"), + coords={"time": times}, + uxgrid=uxgrid, + ) + + res = uxda.subset.bounding_box(lon_bounds=(-10, 10), lat_bounds=(-10, 10)) + + assert isinstance(res, ux.UxDataArray) + assert res.sizes["time"] == times.size + assert "n_face" in res.dims diff --git a/uxarray/grid/grid.py b/uxarray/grid/grid.py index ab9677e49..2a2580ce9 100644 --- a/uxarray/grid/grid.py +++ b/uxarray/grid/grid.py @@ -66,7 +66,7 @@ _populate_edge_node_distances, ) from uxarray.grid.point_in_face import _point_in_face_query -from uxarray.grid.utils import make_setter +from uxarray.grid.utils import _drop_non_grid_coords, make_setter from uxarray.grid.validation import ( _check_area, _check_connectivity, @@ -108,18 +108,6 @@ from uxarray.core.dataarray import UxDataArray -def _drop_non_grid_coords(ds): - """Drop coordinates that aren't recognized grid coordinates (e.g. a stray ``time`` - carried in from the source file). - - Coordinate-only, so grid data variables — connectivity, descriptors, and the - subset's ``subgrid_*_indices`` — are always left intact. - """ - grid_coords = set(ugrid.SPHERICAL_COORD_NAMES) | set(ugrid.CARTESIAN_COORD_NAMES) - stray = [coord for coord in ds.coords if coord not in grid_coords] - return ds.drop_vars(stray, errors="ignore") - - class Grid: """Represents a two-dimensional unstructured grid encoded following the UGRID conventions and provides grid-specific functionality. diff --git a/uxarray/grid/utils.py b/uxarray/grid/utils.py index a33f50629..f99ca678c 100644 --- a/uxarray/grid/utils.py +++ b/uxarray/grid/utils.py @@ -3,6 +3,7 @@ from numba import njit, prange from uxarray.constants import INT_FILL_VALUE +from uxarray.conventions import ugrid from uxarray.utils.numba_math import ( _numba_add3, _numba_mul3_scalar, @@ -11,6 +12,18 @@ ) +def _drop_non_grid_coords(ds): + """Drop coordinates that aren't recognized grid coordinates (e.g. a stray ``time`` + carried in from the source file). + + Coordinate-only, so grid data variables — connectivity, descriptors, and the + subset's ``subgrid_*_indices`` — are always left intact. + """ + grid_coords = set(ugrid.SPHERICAL_COORD_NAMES) | set(ugrid.CARTESIAN_COORD_NAMES) + stray = [coord for coord in ds.coords if coord not in grid_coords] + return ds.drop_vars(stray, errors="ignore") + + @njit(cache=True) def _small_angle_of_2_vectors(u, v): """ From 4786101c44164ffc414a600c3b72b678dea6d86f Mon Sep 17 00:00:00 2001 From: Dylan Nelson <44685561+dylannelson@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:55:48 -0400 Subject: [PATCH 7/7] size change to dropping coordinates --- uxarray/grid/grid.py | 4 ++-- uxarray/grid/utils.py | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/uxarray/grid/grid.py b/uxarray/grid/grid.py index 2a2580ce9..39e181877 100644 --- a/uxarray/grid/grid.py +++ b/uxarray/grid/grid.py @@ -192,8 +192,8 @@ def __init__( self.source_grid_spec = source_grid_spec # internal xarray dataset for storing grid variables. - # drop stray coordinates (e.g. a `time` carried in from the source file) so they - # can't leak onto the grid and collide during subsetting (see #1444). + # drop stray scalar coordinates (e.g. a `time` carried in from the source file) + # so they can't leak onto the grid and collide during subsetting (see #1444). self._ds = _drop_non_grid_coords(grid_ds) # source grid specification (i.e. UGRID, MPAS, SCRIP, etc.) diff --git a/uxarray/grid/utils.py b/uxarray/grid/utils.py index f99ca678c..5b3706012 100644 --- a/uxarray/grid/utils.py +++ b/uxarray/grid/utils.py @@ -13,14 +13,23 @@ def _drop_non_grid_coords(ds): - """Drop coordinates that aren't recognized grid coordinates (e.g. a stray ``time`` - carried in from the source file). + """Drop scalar (0-d) coordinates that aren't recognized grid coordinates + (e.g. a stray ``time`` carried in from the source file). + + Only 0-d strays can cause the subset collision behind issue #1444: a scalar + coordinate attaches to every variable, including the ``(n_face,)`` + ``subgrid_*_indices`` indexer, where it can clash with a like-named dimension + on the data being indexed. A coordinate with its own dimension (e.g. ICON's + 1-d ``clon``/``clat`` or FESOM2's ``lon``/``lat``) never attaches to that + indexer, so it is left in place and those grids round-trip unchanged. Coordinate-only, so grid data variables — connectivity, descriptors, and the subset's ``subgrid_*_indices`` — are always left intact. """ grid_coords = set(ugrid.SPHERICAL_COORD_NAMES) | set(ugrid.CARTESIAN_COORD_NAMES) - stray = [coord for coord in ds.coords if coord not in grid_coords] + stray = [ + coord for coord in ds.coords if coord not in grid_coords and ds[coord].ndim == 0 + ] return ds.drop_vars(stray, errors="ignore")