diff --git a/docs/changelog.rst b/docs/changelog.rst index 81d77bfe4..7c3bdc410 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -36,7 +36,10 @@ Fixed - :class:`xugrid.CentroidLocatorRegridder`, :class:`xugrid.OverlapRegridder`, and :class:`xugrid.BarycentricInterpolator` will now also regrid structured to unstructured grid topologies. - +- :meth:`xugrid.Ugrid1d.to_dataset` and :meth:`xugrid.Ugrid2d.to_dataset` no + longer write unused connectivity variables into the attributes of the UGRID + dummy variable. + Changed ~~~~~~~ diff --git a/tests/test_conventions.py b/tests/test_conventions.py index a0d92a646..06e3db829 100644 --- a/tests/test_conventions.py +++ b/tests/test_conventions.py @@ -294,3 +294,40 @@ def test_dimensions(self): def test_connectivity(self): assert self.ds.ugrid_roles.connectivity == self.connectivity + + def test_dimension_name_mismatch_error(self): + ds = self.ds.copy() + + ds["mesh2d_edge_nodes"] = xr.DataArray( + data=[ + [0, 1], + [1, 2], + [2, 3], + [3, 0], + ], + dims=["nEdges", "Two"], + attrs={"_FillValue": -1, "start_index": 0}, + ) + + with pytest.raises( + cv.UgridDimensionError, + match="edge_dimension: nEdges not in edge_face_connectivity", + ): + ds.ugrid_roles.dimensions + + def test_dimension_size_error(self): + ds = self.ds.copy() + + ds["mesh2d_edge_nodes"] = xr.DataArray( + data=[ + [0, 1, -1], + [1, 2, -1], + [2, 3, -1], + [3, 0, -1], + ], + dims=["mesh2d_nEdges", "Three"], + attrs={"_FillValue": -1, "start_index": 0}, + ) + + with pytest.raises(cv.UgridDimensionError, match="Expected size 2"): + ds.ugrid_roles.dimensions diff --git a/tests/test_ugrid1d.py b/tests/test_ugrid1d.py index ad7d8ccd7..0ec3fefc3 100644 --- a/tests/test_ugrid1d.py +++ b/tests/test_ugrid1d.py @@ -157,6 +157,17 @@ def test_to_crs(): def test_to_dataset(): + def check_attrs(ds): + attrs = ds[NAME].attrs.copy() + attrs.pop("cf_role") + attrs.pop("long_name") + attrs.pop("topology_dimension") + ds_contents = tuple(ds.dims) + tuple(ds.coords) + tuple(ds.data_vars) + for values in attrs.values(): + # e.g node_coordinates are joined by a whitespace. + for value in values.split(" "): + assert value in ds_contents + grid = grid1d() ds = grid.to_dataset() assert isinstance(ds, xr.Dataset) @@ -166,10 +177,12 @@ def test_to_dataset(): assert f"{NAME}_node_x" in ds.coords assert f"{NAME}_node_y" in ds.coords assert f"{NAME}_edge_nodes" in ds + check_attrs(ds) ds = grid.to_dataset(optional_attributes=True) assert f"{NAME}_edge_x" in ds.coords assert f"{NAME}_edge_y" in ds.coords + check_attrs(ds) def test_ugrid1d_dataset_roundtrip(): diff --git a/tests/test_ugrid2d.py b/tests/test_ugrid2d.py index 43f7ae6b2..2d7c79f3c 100644 --- a/tests/test_ugrid2d.py +++ b/tests/test_ugrid2d.py @@ -215,6 +215,17 @@ def test_to_crs(): def test_to_dataset(): + def check_attrs(ds): + attrs = ds[NAME].attrs.copy() + attrs.pop("cf_role") + attrs.pop("long_name") + attrs.pop("topology_dimension") + ds_contents = tuple(ds.dims) + tuple(ds.coords) + tuple(ds.data_vars) + for values in attrs.values(): + # e.g node_coordinates are joined by a whitespace. + for value in values.split(" "): + assert value in ds_contents + grid = grid2d() ds = grid.to_dataset() assert isinstance(ds, xr.Dataset) @@ -224,6 +235,7 @@ def test_to_dataset(): assert f"{NAME}_node_x" in ds.coords assert f"{NAME}_node_y" in ds.coords assert f"{NAME}_face_nodes" in ds + check_attrs(ds) ds = grid.to_dataset(optional_attributes=True) assert f"{NAME}_edge_nodes" in ds @@ -236,6 +248,7 @@ def test_to_dataset(): assert f"{NAME}_face_y" in ds assert f"{NAME}_edge_x" in ds assert f"{NAME}_edge_y" in ds + check_attrs(ds) def test_ugrid2d_set_node_coords(): diff --git a/xugrid/ugrid/conventions.py b/xugrid/ugrid/conventions.py index fe82691fa..0a3bbd64a 100644 --- a/xugrid/ugrid/conventions.py +++ b/xugrid/ugrid/conventions.py @@ -40,9 +40,9 @@ class UgridCoordinateError(Exception): "face_node_connectivity", "edge_node_connectivity", "face_edge_connectivity", - # "face_face_connectivity", + "face_face_connectivity", "edge_face_connectivity", - # "boundary_node_connectivity", + "boundary_node_connectivity", ), } @@ -50,9 +50,9 @@ class UgridCoordinateError(Exception): "face_node_connectivity": ("face_dimension", None), "edge_node_connectivity": ("edge_dimension", 2), "face_edge_connectivity": ("face_dimension", None), - # "face_face_connectivity": ("face_dimension", None), + "face_face_connectivity": ("face_dimension", None), "edge_face_connectivity": ("edge_dimension", 2), - # "boundary_node_connectivity": ("???","???"), + "boundary_node_connectivity": ("boundary_edge_dimension", 2), } X_STANDARD_NAMES = ("projection_x_coordinate", "longitude") @@ -261,7 +261,7 @@ def _infer_dims( expected_dims = _CONNECTIVITY_DIMS[role] var_dims = ds[varname].dims for key, dim in zip(expected_dims, var_dims): - if isinstance(key, str): # skip None or integer + if isinstance(key, str): prev_dim = inferred.get(key) # Not specified: default order can be used to infer dimensions. if prev_dim is None: @@ -272,6 +272,16 @@ def _infer_dims( f"{key}: {prev_dim} not in {role}: {varname}" f" with dimensions: {var_dims}" ) + elif isinstance(key, int): + dim_size = ds.dims[dim] + if not dim_size == key: + raise UgridDimensionError( + f"Expected size {key} for dimension {dim} in variable " + f"{varname} with role {role}, found instead: " + f"{dim_size}" + ) + + # If key is None, we don't do any checking. for role, varnames in coordinates.items(): key = _COORD_DIMS[role] diff --git a/xugrid/ugrid/ugridbase.py b/xugrid/ugrid/ugridbase.py index ac28841bf..f8695f652 100644 --- a/xugrid/ugrid/ugridbase.py +++ b/xugrid/ugrid/ugridbase.py @@ -227,15 +227,16 @@ def _filtered_attrs(self, dataset: xr.Dataset): topodim = self.topology_dimension attrs = self._attrs.copy() - for key in conventions._DIM_NAMES[topodim]: - if key in attrs: - if attrs[key] not in dataset.dims: - attrs.pop(key) + ugrid_dims = conventions._DIM_NAMES[topodim] + tuple( + [dims[0] for dims in conventions._CONNECTIVITY_DIMS.values()] + ) + for key in ugrid_dims: + if key in attrs and attrs[key] not in dataset.dims: + attrs.pop(key) for key in conventions._CONNECTIVITY_NAMES[topodim]: - if key in attrs: - if attrs[key] not in dataset: - attrs.pop(key) + if key in attrs and attrs[key] not in dataset: + attrs.pop(key) for coord in conventions._COORD_NAMES[topodim]: if coord in attrs: