From 3663317f4e9159208dfeb8730bf4e54e19e45029 Mon Sep 17 00:00:00 2001 From: Chris Barker Date: Fri, 6 Feb 2026 13:26:10 -0800 Subject: [PATCH 1/2] add check for invalid start_index when inferring. --- tests/test_grids/test_ugrid.py | 16 ++++++++++++++++ xarray_subset_grid/grids/ugrid.py | 6 +++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/test_grids/test_ugrid.py b/tests/test_grids/test_ugrid.py index ebfe811..096dcd1 100644 --- a/tests/test_grids/test_ugrid.py +++ b/tests/test_grids/test_ugrid.py @@ -419,6 +419,22 @@ def test_assign_ugrid_topology_start_index_zero_infer(): assert ds["mesh_edge_nodes"].attrs["start_index"] == 0 assert ds["mesh_boundary_nodes"].attrs["start_index"] == 0 +def test_assign_ugrid_topology_start_index_zero_infer_bad_data(): + """ + if the connectivity array has negative or no zero or one indexes -- not good. + """ + # set a negative index + ds = xr.open_dataset(EXAMPLE_DATA / "small_ugrid_zero_based.nc") + faces = ds['mesh_face_nodes'].data[0, 0] = -3 + with pytest.raises(ValueError): + ds = ugrid.assign_ugrid_topology(ds, face_node_connectivity="mesh_face_nodes") + + # remove zero and one indexes + data = ds['mesh_face_nodes'].data + data[data < 2] = 100 + with pytest.raises(ValueError): + ds = ugrid.assign_ugrid_topology(ds, face_node_connectivity="mesh_face_nodes") + # NOTE: these tests are probably not complete -- but they are something. # we really should have a complete UGRID example to test with. diff --git a/xarray_subset_grid/grids/ugrid.py b/xarray_subset_grid/grids/ugrid.py index 436f924..5b7bc43 100644 --- a/xarray_subset_grid/grids/ugrid.py +++ b/xarray_subset_grid/grids/ugrid.py @@ -463,8 +463,12 @@ def assign_ugrid_topology( mapping = [(dim, ds[dim].size) for dim in dims] mesh.face_dimension = next(x[0] for x in mapping if x[1] != 3 and x[1] != 4) - # check for start_index, and set it if there. + # check for start_index, and set it not there. if mesh.start_index is None: + start_index = int(ds[mesh.face_node_connectivity].min()) + if start_index not in (0, 1): + raise ValueError(f"minimum index in face_node_connectivity array is {start_index}" + " -- it should be zero (C-style indexing) or 1 (Fortran-style indexing)") mesh.start_index = int(ds[mesh.face_node_connectivity].min()) if mesh.start_index not in (0, 1): From 1a3515ae03132ce0f640127b121edda3ffd799ac Mon Sep 17 00:00:00 2001 From: Filipe Fernandes Date: Tue, 7 Jul 2026 10:38:18 -0300 Subject: [PATCH 2/2] lints --- tests/test_grids/test_ugrid.py | 5 +++-- xarray_subset_grid/grids/ugrid.py | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_grids/test_ugrid.py b/tests/test_grids/test_ugrid.py index 096dcd1..a34c71c 100644 --- a/tests/test_grids/test_ugrid.py +++ b/tests/test_grids/test_ugrid.py @@ -419,18 +419,19 @@ def test_assign_ugrid_topology_start_index_zero_infer(): assert ds["mesh_edge_nodes"].attrs["start_index"] == 0 assert ds["mesh_boundary_nodes"].attrs["start_index"] == 0 + def test_assign_ugrid_topology_start_index_zero_infer_bad_data(): """ if the connectivity array has negative or no zero or one indexes -- not good. """ # set a negative index ds = xr.open_dataset(EXAMPLE_DATA / "small_ugrid_zero_based.nc") - faces = ds['mesh_face_nodes'].data[0, 0] = -3 + ds["mesh_face_nodes"].data[0, 0] = -3 with pytest.raises(ValueError): ds = ugrid.assign_ugrid_topology(ds, face_node_connectivity="mesh_face_nodes") # remove zero and one indexes - data = ds['mesh_face_nodes'].data + data = ds["mesh_face_nodes"].data data[data < 2] = 100 with pytest.raises(ValueError): ds = ugrid.assign_ugrid_topology(ds, face_node_connectivity="mesh_face_nodes") diff --git a/xarray_subset_grid/grids/ugrid.py b/xarray_subset_grid/grids/ugrid.py index 5b7bc43..5e072ec 100644 --- a/xarray_subset_grid/grids/ugrid.py +++ b/xarray_subset_grid/grids/ugrid.py @@ -467,8 +467,10 @@ def assign_ugrid_topology( if mesh.start_index is None: start_index = int(ds[mesh.face_node_connectivity].min()) if start_index not in (0, 1): - raise ValueError(f"minimum index in face_node_connectivity array is {start_index}" - " -- it should be zero (C-style indexing) or 1 (Fortran-style indexing)") + raise ValueError( + f"minimum index in face_node_connectivity array is {start_index}" + " -- it should be zero (C-style indexing) or 1 (Fortran-style indexing)" + ) mesh.start_index = int(ds[mesh.face_node_connectivity].min()) if mesh.start_index not in (0, 1):