Skip to content

Commit

Permalink
Remove connectivity dimensions from attributes as well. Fixes #132.
Browse files Browse the repository at this point in the history
This also adds a check on the dimensions sizes of connectivity variables, and some unit tests for triggering the errors.
  • Loading branch information
Huite committed Aug 8, 2023
1 parent b6c4fb5 commit 75d5ff1
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 13 deletions.
5 changes: 4 additions & 1 deletion docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~

Expand Down
37 changes: 37 additions & 0 deletions tests/test_conventions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 13 additions & 0 deletions tests/test_ugrid1d.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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():
Expand Down
13 changes: 13 additions & 0 deletions tests/test_ugrid2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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():
Expand Down
20 changes: 15 additions & 5 deletions xugrid/ugrid/conventions.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ 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",
),
}

_CONNECTIVITY_DIMS = {
"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")
Expand Down Expand Up @@ -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:
Expand All @@ -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]
Expand Down
15 changes: 8 additions & 7 deletions xugrid/ugrid/ugridbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 75d5ff1

Please sign in to comment.