From 2242b105fe49e304ac324cbc71773be5ea3c194f Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:06:36 +0100 Subject: [PATCH 01/13] Add datasets_sgrid --- src/parcels/_datasets/structured/generic.py | 42 +++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/parcels/_datasets/structured/generic.py b/src/parcels/_datasets/structured/generic.py index 7758cfe18..82be61fa0 100644 --- a/src/parcels/_datasets/structured/generic.py +++ b/src/parcels/_datasets/structured/generic.py @@ -1,6 +1,8 @@ import numpy as np import xarray as xr +from parcels._core.utils.sgrid import DimDimPadding, Grid2DMetadata, Grid3DMetadata, Padding + from . import T, X, Y, Z __all__ = ["T", "X", "Y", "Z", "datasets"] @@ -8,6 +10,17 @@ TIME = xr.date_range("2000", "2001", T) +def _copy_and_attach_sgrid_metadata(ds, grid: Grid2DMetadata | Grid3DMetadata): + ds = ds.copy() + ds["grid"] = ( + [], + 0, + grid.to_attrs(), + ) + ds.attrs["conventions"] = "SGRID" + return ds + + def _rotated_curvilinear_grid(): XG = np.arange(X) YG = np.arange(Y) @@ -225,3 +238,32 @@ def _unrolled_cone_curvilinear_grid(): ), "2d_left_unrolled_cone": _unrolled_cone_curvilinear_grid(), } + +datasets_sgrid = { + "ds_2d_left": datasets["ds_2d_left"].pipe( + _copy_and_attach_sgrid_metadata, + Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("YG", "XG"), + face_dimensions=( + DimDimPadding("YC", "YG", Padding.HIGH), + DimDimPadding("XC", "XG", Padding.HIGH), + ), + vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), + ), + ), + "ds_2d_right": datasets["ds_2d_right"].pipe( + _copy_and_attach_sgrid_metadata, + Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("YG", "XG"), + face_dimensions=( + DimDimPadding("YC", "YG", Padding.HIGH), + DimDimPadding("XC", "XG", Padding.HIGH), + ), + vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), + ), + ), +} From 5239033a14cc335e0ff8060f32046f2542acfda1 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:28:31 +0100 Subject: [PATCH 02/13] Add reprs for sgrid grid metadata objects --- src/parcels/_core/utils/sgrid.py | 8 ++++++++ src/parcels/_python.py | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/src/parcels/_core/utils/sgrid.py b/src/parcels/_core/utils/sgrid.py index 825649cf2..5ad10ba95 100644 --- a/src/parcels/_core/utils/sgrid.py +++ b/src/parcels/_core/utils/sgrid.py @@ -19,6 +19,8 @@ import xarray as xr +from parcels._python import _repr_from_dunder_dict + RE_DIM_DIM_PADDING = r"(\w+):(\w+)\s*\(padding:\s*(\w+)\)" Dim = str @@ -94,6 +96,9 @@ def __init__( #! Important optional attribute for 2D grids with vertical layering self.vertical_dimensions = vertical_dimensions + def __repr__(self) -> str: + return _repr_from_dunder_dict(self) + def __eq__(self, other: Any) -> bool: if not isinstance(other, Grid2DMetadata): return NotImplemented @@ -180,6 +185,9 @@ def __init__( # face *i_coordinates* # volume_coordinates + def __repr__(self) -> str: + return _repr_from_dunder_dict(self) + def __eq__(self, other: Any) -> bool: if not isinstance(other, Grid3DMetadata): return NotImplemented diff --git a/src/parcels/_python.py b/src/parcels/_python.py index a78e9bf0d..02fd6ea79 100644 --- a/src/parcels/_python.py +++ b/src/parcels/_python.py @@ -14,6 +14,12 @@ def isinstance_noimport(obj, class_or_tuple): ) +def _repr_from_dunder_dict(obj: object) -> str: + """Dataclass-like __repr__ implementation based on __dict__.""" + parts = [f"{k}={v!r}" for k, v in obj.__dict__.items()] + return f"{obj.__class__.__qualname__}(" + ", ".join(parts) + ")" + + def assert_same_function_signature(f: Callable, *, ref: Callable, context: str) -> None: """Ensures a function `f` has the same signature as the reference function `ref`.""" sig_ref = inspect.signature(ref) From bdd4f84a6fd400c8f748619e24fe5fb417f7379b Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:02:17 +0100 Subject: [PATCH 03/13] Add SGRID rename dims functionality --- src/parcels/_core/utils/sgrid.py | 98 ++++++++++++++++++++++++ tests/utils/test_sgrid.py | 127 ++++++++++++++++++++++++++----- 2 files changed, 204 insertions(+), 21 deletions(-) diff --git a/src/parcels/_core/utils/sgrid.py b/src/parcels/_core/utils/sgrid.py index 5ad10ba95..f345b98a4 100644 --- a/src/parcels/_core/utils/sgrid.py +++ b/src/parcels/_core/utils/sgrid.py @@ -134,6 +134,9 @@ def to_attrs(self) -> dict[str, str | int]: d["vertical_dimensions"] = dump_mappings(self.vertical_dimensions) return d + def rename_dims(self, dims_dict: dict[str, str]) -> Self: + return _metadata_rename_dims(self, dims_dict) + class Grid3DMetadata(SGridMetadataProtocol): def __init__( @@ -218,6 +221,9 @@ def to_attrs(self) -> dict[str, str | int]: volume_dimensions=dump_mappings(self.volume_dimensions), ) + def rename_dims(self, dims_dict: dict[str, str]) -> Self: + return _metadata_rename_dims(self, dims_dict) + @dataclass class DimDimPadding: @@ -386,3 +392,95 @@ def parse_sgrid(ds: xr.Dataset): xgcm_coords[axis] = {"center": dim_dim_padding.dim2, xgcm_position: dim_dim_padding.dim1} return (ds, {"coords": xgcm_coords}) + + +def rename_dims(ds: xr.Dataset, dims_dict: dict[str, str]) -> xr.Dataset: + grid_da = get_grid_topology(ds) + if grid_da is None: + raise ValueError( + "No variable found in dataset with 'cf_role' attribute set to 'grid_topology'. Is this an SGrid dataset?" + ) + + grid = parse_grid_attrs(grid_da.attrs) + ds = ds.rename_dims(dims_dict) + + # Update the metadata + ds[grid_da.name].attrs = grid.rename_dims(dims_dict).to_attrs() + return ds + + +def _get_unique_dim_names(grid: Grid2DMetadata | Grid3DMetadata) -> set[str]: + dims = set() + dims.update(set(grid.node_dimensions)) + + for key, value in grid.__dict__.items(): + if key in ("cf_role", "topology_dimension") or value is None: + continue + assert isinstance(value, tuple), ( + f"Expected sgrid metadata attribute to be represented as a tuple, got {value!r}. Is '{key}' a valid SGrid metadata attribute?" + ) + for item in value: + if isinstance(item, DimDimPadding): + dims.add(item.dim1) + dims.add(item.dim2) + else: + assert isinstance(item, str) + dims.add(item) + return dims + + +@overload +def _metadata_rename_dims(grid: Grid2DMetadata, dims_dict: dict[str, str]) -> Grid2DMetadata: ... + + +@overload +def _metadata_rename_dims(grid: Grid3DMetadata, dims_dict: dict[str, str]) -> Grid3DMetadata: ... + + +def _metadata_rename_dims(grid, dims_dict): + """ + Renames dimensions in SGrid metadata. + + Similar in API to xr.Dataset.rename_dims. Renames dimensions according to dims_dict mapping + of old dimension names to new dimension names. + """ + dims_dict = dims_dict.copy() + assert len(dims_dict) == len(set(dims_dict.values())), "dims_dict contains non-unique target dimension names" + + existing_dims = _get_unique_dim_names(grid) + for dim in dims_dict.keys(): + if dim not in existing_dims: + raise ValueError(f"Dimension {dim!r} not found in SGrid metadata dimensions {existing_dims!r}") + + for dim in existing_dims: + if dim not in dims_dict: + dims_dict[dim] = dim # identity mapping for dimensions not being renamed + + kwargs = {} + for key, value in grid.__dict__.items(): + if isinstance(value, tuple): + new_value = [] + for item in value: + if isinstance(item, DimDimPadding): + new_item = DimDimPadding( + dim1=dims_dict[item.dim1], + dim2=dims_dict[item.dim2], + padding=item.padding, + ) + new_value.append(new_item) + else: + assert isinstance(item, str) + new_value.append(dims_dict[item]) + kwargs[key] = tuple(new_value) + continue + + if key in ("cf_role", "topology_dimension") or value is None: + kwargs[key] = value + continue + + if isinstance(value, str): + kwargs[key] = dims_dict[value] + continue + + raise ValueError(f"Unexpected attribute {key!r} on {grid!r}") + return type(grid)(**kwargs) diff --git a/tests/utils/test_sgrid.py b/tests/utils/test_sgrid.py index c53d17512..056c9c3a9 100644 --- a/tests/utils/test_sgrid.py +++ b/tests/utils/test_sgrid.py @@ -3,30 +3,26 @@ import xarray as xr import xgcm from hypothesis import assume, example, given +from hypothesis import strategies as st from parcels._core.utils import sgrid from tests.strategies import sgrid as sgrid_strategies -def get_unique_dim_names(grid: sgrid.Grid2DMetadata | sgrid.Grid3DMetadata) -> set[str]: - dims = set() - dims.update(set(grid.node_dimensions)) - - for value in [ - grid.node_dimensions, - grid.face_dimensions if isinstance(grid, sgrid.Grid2DMetadata) else grid.volume_dimensions, - grid.vertical_dimensions if isinstance(grid, sgrid.Grid2DMetadata) else None, - ]: - if value is None: - continue - for item in value: - if isinstance(item, sgrid.DimDimPadding): - dims.add(item.dim1) - dims.add(item.dim2) - else: - assert isinstance(item, str) - dims.add(item) - return dims +@pytest.fixture +def grid2dmetadata(): + return sgrid.Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("node_dimension1", "node_dimension2"), + face_dimensions=( + sgrid.DimDimPadding("face_dimension1", "node_dimension1", sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension2", "node_dimension2", sgrid.Padding.LOW), + ), + vertical_dimensions=( + sgrid.DimDimPadding("vertical_dimensions_dim1", "vertical_dimensions_dim2", sgrid.Padding.LOW), + ), + ) def dummy_sgrid_ds(grid: sgrid.Grid2DMetadata | sgrid.Grid3DMetadata) -> xr.Dataset: @@ -42,7 +38,7 @@ def dummy_sgrid_2d_ds(grid: sgrid.Grid2DMetadata) -> xr.Dataset: ds = dummy_comodo_3d_ds() # Can't rename dimensions that already exist in the dataset - assume(get_unique_dim_names(grid) & set(ds.dims) == set()) + assume(sgrid._get_unique_dim_names(grid) & set(ds.dims) == set()) renamings = {} if grid.vertical_dimensions is None: @@ -67,7 +63,7 @@ def dummy_sgrid_3d_ds(grid: sgrid.Grid3DMetadata) -> xr.Dataset: ds = dummy_comodo_3d_ds() # Can't rename dimensions that already exist in the dataset - assume(get_unique_dim_names(grid) & set(ds.dims) == set()) + assume(sgrid._get_unique_dim_names(grid) & set(ds.dims) == set()) renamings = {} for old, new in zip(["XG", "YG", "ZG"], grid.node_dimensions, strict=True): @@ -240,3 +236,92 @@ def test_parse_sgrid_3d(grid_metadata: sgrid.Grid3DMetadata): coords = grid.axes[axis].coords assert coords["center"] == dim_edge assert coords[sgrid.SGRID_PADDING_TO_XGCM_POSITION[padding]] == dim_node + + +@example( + grids_and_dims_dict=( + sgrid.Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("node_dimension1", "node_dimension2"), + face_dimensions=( + sgrid.DimDimPadding("face_dimension1", "node_dimension1", sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension2", "node_dimension2", sgrid.Padding.LOW), + ), + vertical_dimensions=( + sgrid.DimDimPadding("vertical_dimensions_dim1", "vertical_dimensions_dim2", sgrid.Padding.LOW), + ), + ), + sgrid.Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("new_node_dimension1", "new_node_dimension2"), + face_dimensions=( + sgrid.DimDimPadding("new_face_dimension1", "new_node_dimension1", sgrid.Padding.LOW), + sgrid.DimDimPadding("new_face_dimension2", "new_node_dimension2", sgrid.Padding.LOW), + ), + vertical_dimensions=( + sgrid.DimDimPadding("new_vertical_dimensions_dim1", "new_vertical_dimensions_dim2", sgrid.Padding.LOW), + ), + ), + { + "node_dimension1": "new_node_dimension1", + "node_dimension2": "new_node_dimension2", + "face_dimension1": "new_face_dimension1", + "face_dimension2": "new_face_dimension2", + "vertical_dimensions_dim1": "new_vertical_dimensions_dim1", + "vertical_dimensions_dim2": "new_vertical_dimensions_dim2", + }, + ) +) +@given( + grids_and_dims_dict=st.lists(sgrid_strategies.dimension_name, min_size=12, max_size=12, unique=True).map( + lambda dims: ( + sgrid.Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=(dims[0], dims[1]), + face_dimensions=( + sgrid.DimDimPadding(dims[2], dims[0], sgrid.Padding.LOW), + sgrid.DimDimPadding(dims[3], dims[1], sgrid.Padding.LOW), + ), + vertical_dimensions=(sgrid.DimDimPadding(dims[4], dims[5], sgrid.Padding.LOW),), + ), + sgrid.Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=(dims[6 + 0], dims[6 + 1]), + face_dimensions=( + sgrid.DimDimPadding(dims[6 + 2], dims[6 + 0], sgrid.Padding.LOW), + sgrid.DimDimPadding(dims[6 + 3], dims[6 + 1], sgrid.Padding.LOW), + ), + vertical_dimensions=(sgrid.DimDimPadding(dims[6 + 4], dims[6 + 5], sgrid.Padding.LOW),), + ), + {k: v for k, v in zip(dims[:6], dims[6:], strict=True)}, + ) + ) +) +def test_rename_dims(grids_and_dims_dict): + """Creates two SGrid 2D metadata objects with disjoint dimension names, and a mapping between the dimension names. + Renames the dimensions of the first grid according to the mapping, and checks that the result + """ + grid_old, grid_new, dims_dict = grids_and_dims_dict + assert grid_old.rename_dims(dims_dict).to_attrs() == grid_new.to_attrs() + + +def test_rename_dims_errors(grid2dmetadata): + # Test various error modes of rename_dims + grid = grid2dmetadata + # Non-unique target dimension names + dims_dict = { + "node_dimension1": "new_node_dimension", + "node_dimension2": "new_node_dimension", + } + with pytest.raises(AssertionError, match="dims_dict contains non-unique target dimension names"): + grid.rename_dims(dims_dict) + # Unexpected attribute in dims_dict + dims_dict = { + "unexpected_dimension": "new_unexpected_dimension", + } + with pytest.raises(ValueError, match="Dimension 'unexpected_dimension' not found in SGrid metadata dimensions"): + grid.rename_dims(dims_dict) From 3b0cfbf4733e4a7e8398af713acc1aa2211599fa Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:05:11 +0100 Subject: [PATCH 04/13] Update name of protocol --- src/parcels/_core/utils/sgrid.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/parcels/_core/utils/sgrid.py b/src/parcels/_core/utils/sgrid.py index f345b98a4..6a4f87de2 100644 --- a/src/parcels/_core/utils/sgrid.py +++ b/src/parcels/_core/utils/sgrid.py @@ -33,12 +33,12 @@ class Padding(enum.Enum): BOTH = "both" -class SGridMetadataProtocol(Protocol): +class AttrsSerializable(Protocol): def to_attrs(self) -> dict[str, str | int]: ... def from_attrs(cls, d: dict[str, Hashable]) -> Self: ... -class Grid2DMetadata(SGridMetadataProtocol): +class Grid2DMetadata(AttrsSerializable): def __init__( self, cf_role: Literal["grid_topology"], @@ -138,7 +138,7 @@ def rename_dims(self, dims_dict: dict[str, str]) -> Self: return _metadata_rename_dims(self, dims_dict) -class Grid3DMetadata(SGridMetadataProtocol): +class Grid3DMetadata(AttrsSerializable): def __init__( self, cf_role: Literal["grid_topology"], From 8de7995c33f4e66ee628abe29547a2d9cd2f0f4d Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:12:59 +0100 Subject: [PATCH 05/13] Rename dimensions in datasets_sgrid So that they align with the conventions document --- src/parcels/_datasets/structured/generic.py | 32 +++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/parcels/_datasets/structured/generic.py b/src/parcels/_datasets/structured/generic.py index 82be61fa0..4dbc80bbb 100644 --- a/src/parcels/_datasets/structured/generic.py +++ b/src/parcels/_datasets/structured/generic.py @@ -1,7 +1,15 @@ import numpy as np import xarray as xr -from parcels._core.utils.sgrid import DimDimPadding, Grid2DMetadata, Grid3DMetadata, Padding +from parcels._core.utils.sgrid import ( + DimDimPadding, + Grid2DMetadata, + Grid3DMetadata, + Padding, +) +from parcels._core.utils.sgrid import ( + rename_dims as sgrid_rename_dims, +) from . import T, X, Y, Z @@ -239,8 +247,17 @@ def _unrolled_cone_curvilinear_grid(): "2d_left_unrolled_cone": _unrolled_cone_curvilinear_grid(), } +_COMODO_TO_2D_SGRID = { # Note "2D SGRID" here is meant in the context of Grid2DMetadata and SGRID convention (i.e., 1D depth) + "XG": "node_dimension1", + "YG": "node_dimension2", + "XC": "face_dimension1", + "YC": "face_dimension2", + "ZG": "vertical_dimensions_dim1", + "ZC": "vertical_dimensions_dim2", +} datasets_sgrid = { - "ds_2d_left": datasets["ds_2d_left"].pipe( + "ds_2d_left": datasets["ds_2d_left"] + .pipe( _copy_and_attach_sgrid_metadata, Grid2DMetadata( cf_role="grid_topology", @@ -252,8 +269,13 @@ def _unrolled_cone_curvilinear_grid(): ), vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), ), + ) + .pipe( + sgrid_rename_dims, + _COMODO_TO_2D_SGRID, ), - "ds_2d_right": datasets["ds_2d_right"].pipe( + "ds_2d_right": datasets["ds_2d_right"] + .pipe( _copy_and_attach_sgrid_metadata, Grid2DMetadata( cf_role="grid_topology", @@ -265,5 +287,9 @@ def _unrolled_cone_curvilinear_grid(): ), vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), ), + ) + .pipe( + sgrid_rename_dims, + _COMODO_TO_2D_SGRID, ), } From 5687dc1c8184fa7c3dca03ba7649150c7cff2e54 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:15:30 +0100 Subject: [PATCH 06/13] Formatting --- src/parcels/_datasets/structured/generic.py | 71 +++++++++++---------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/parcels/_datasets/structured/generic.py b/src/parcels/_datasets/structured/generic.py index 4dbc80bbb..31f20e015 100644 --- a/src/parcels/_datasets/structured/generic.py +++ b/src/parcels/_datasets/structured/generic.py @@ -18,7 +18,8 @@ TIME = xr.date_range("2000", "2001", T) -def _copy_and_attach_sgrid_metadata(ds, grid: Grid2DMetadata | Grid3DMetadata): +def _attach_sgrid_metadata(ds, grid: Grid2DMetadata | Grid3DMetadata): + """Copies the dataset and attaches the SGRID metadata in 'grid' variable. Modifies 'conventions' attribute.""" ds = ds.copy() ds["grid"] = ( [], @@ -256,40 +257,44 @@ def _unrolled_cone_curvilinear_grid(): "ZC": "vertical_dimensions_dim2", } datasets_sgrid = { - "ds_2d_left": datasets["ds_2d_left"] - .pipe( - _copy_and_attach_sgrid_metadata, - Grid2DMetadata( - cf_role="grid_topology", - topology_dimension=2, - node_dimensions=("YG", "XG"), - face_dimensions=( - DimDimPadding("YC", "YG", Padding.HIGH), - DimDimPadding("XC", "XG", Padding.HIGH), + "ds_2d_left": ( + datasets["ds_2d_left"] + .pipe( + _attach_sgrid_metadata, + Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("YG", "XG"), + face_dimensions=( + DimDimPadding("YC", "YG", Padding.HIGH), + DimDimPadding("XC", "XG", Padding.HIGH), + ), + vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), ), - vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), - ), - ) - .pipe( - sgrid_rename_dims, - _COMODO_TO_2D_SGRID, + ) + .pipe( + sgrid_rename_dims, + _COMODO_TO_2D_SGRID, + ) ), - "ds_2d_right": datasets["ds_2d_right"] - .pipe( - _copy_and_attach_sgrid_metadata, - Grid2DMetadata( - cf_role="grid_topology", - topology_dimension=2, - node_dimensions=("YG", "XG"), - face_dimensions=( - DimDimPadding("YC", "YG", Padding.HIGH), - DimDimPadding("XC", "XG", Padding.HIGH), + "ds_2d_right": ( + datasets["ds_2d_right"] + .pipe( + _attach_sgrid_metadata, + Grid2DMetadata( + cf_role="grid_topology", + topology_dimension=2, + node_dimensions=("YG", "XG"), + face_dimensions=( + DimDimPadding("YC", "YG", Padding.HIGH), + DimDimPadding("XC", "XG", Padding.HIGH), + ), + vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), ), - vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), - ), - ) - .pipe( - sgrid_rename_dims, - _COMODO_TO_2D_SGRID, + ) + .pipe( + sgrid_rename_dims, + _COMODO_TO_2D_SGRID, + ) ), } From 9f74a6d753cf6587d30c04e8e4cec38277350aa6 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:46:52 +0100 Subject: [PATCH 07/13] Self review --- src/parcels/_core/utils/sgrid.py | 32 ++++++++++----------- src/parcels/_datasets/structured/generic.py | 10 +++---- src/parcels/_python.py | 2 +- tests/utils/test_sgrid.py | 5 ++-- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/parcels/_core/utils/sgrid.py b/src/parcels/_core/utils/sgrid.py index 6a4f87de2..d2c8e6b4c 100644 --- a/src/parcels/_core/utils/sgrid.py +++ b/src/parcels/_core/utils/sgrid.py @@ -19,7 +19,7 @@ import xarray as xr -from parcels._python import _repr_from_dunder_dict +from parcels._python import repr_from_dunder_dict RE_DIM_DIM_PADDING = r"(\w+):(\w+)\s*\(padding:\s*(\w+)\)" @@ -33,6 +33,15 @@ class Padding(enum.Enum): BOTH = "both" +SGRID_PADDING_TO_XGCM_POSITION = { + Padding.LOW: "right", + Padding.HIGH: "left", + Padding.BOTH: "inner", + Padding.NONE: "outer", + # "center" position is not used in SGrid, in SGrid this would just be the edges/faces themselves +} + + class AttrsSerializable(Protocol): def to_attrs(self) -> dict[str, str | int]: ... def from_attrs(cls, d: dict[str, Hashable]) -> Self: ... @@ -97,7 +106,7 @@ def __init__( self.vertical_dimensions = vertical_dimensions def __repr__(self) -> str: - return _repr_from_dunder_dict(self) + return repr_from_dunder_dict(self) def __eq__(self, other: Any) -> bool: if not isinstance(other, Grid2DMetadata): @@ -189,7 +198,7 @@ def __init__( # volume_coordinates def __repr__(self) -> str: - return _repr_from_dunder_dict(self) + return repr_from_dunder_dict(self) def __eq__(self, other: Any) -> bool: if not isinstance(other, Grid3DMetadata): @@ -332,15 +341,6 @@ def maybe_load_mappings(s): return load_mappings(s) -SGRID_PADDING_TO_XGCM_POSITION = { - Padding.LOW: "right", - Padding.HIGH: "left", - Padding.BOTH: "inner", - Padding.NONE: "outer", - # "center" position is not used in SGrid, in SGrid this would just be the edges/faces themselves -} - - class SGridParsingException(Exception): """Exception raised when parsing SGrid attributes fails.""" @@ -401,15 +401,15 @@ def rename_dims(ds: xr.Dataset, dims_dict: dict[str, str]) -> xr.Dataset: "No variable found in dataset with 'cf_role' attribute set to 'grid_topology'. Is this an SGrid dataset?" ) - grid = parse_grid_attrs(grid_da.attrs) ds = ds.rename_dims(dims_dict) # Update the metadata + grid = parse_grid_attrs(grid_da.attrs) ds[grid_da.name].attrs = grid.rename_dims(dims_dict).to_attrs() return ds -def _get_unique_dim_names(grid: Grid2DMetadata | Grid3DMetadata) -> set[str]: +def get_unique_dim_names(grid: Grid2DMetadata | Grid3DMetadata) -> set[str]: dims = set() dims.update(set(grid.node_dimensions)) @@ -445,9 +445,9 @@ def _metadata_rename_dims(grid, dims_dict): of old dimension names to new dimension names. """ dims_dict = dims_dict.copy() - assert len(dims_dict) == len(set(dims_dict.values())), "dims_dict contains non-unique target dimension names" + assert len(dims_dict) == len(set(dims_dict.values())), "dims_dict contains duplicate target dimension names" - existing_dims = _get_unique_dim_names(grid) + existing_dims = get_unique_dim_names(grid) for dim in dims_dict.keys(): if dim not in existing_dims: raise ValueError(f"Dimension {dim!r} not found in SGrid metadata dimensions {existing_dims!r}") diff --git a/src/parcels/_datasets/structured/generic.py b/src/parcels/_datasets/structured/generic.py index 31f20e015..9f952cd0e 100644 --- a/src/parcels/_datasets/structured/generic.py +++ b/src/parcels/_datasets/structured/generic.py @@ -257,7 +257,7 @@ def _unrolled_cone_curvilinear_grid(): "ZC": "vertical_dimensions_dim2", } datasets_sgrid = { - "ds_2d_left": ( + "ds_2d_padded_high": ( datasets["ds_2d_left"] .pipe( _attach_sgrid_metadata, @@ -277,7 +277,7 @@ def _unrolled_cone_curvilinear_grid(): _COMODO_TO_2D_SGRID, ) ), - "ds_2d_right": ( + "ds_2d_padded_low": ( datasets["ds_2d_right"] .pipe( _attach_sgrid_metadata, @@ -286,10 +286,10 @@ def _unrolled_cone_curvilinear_grid(): topology_dimension=2, node_dimensions=("YG", "XG"), face_dimensions=( - DimDimPadding("YC", "YG", Padding.HIGH), - DimDimPadding("XC", "XG", Padding.HIGH), + DimDimPadding("YC", "YG", Padding.LOW), + DimDimPadding("XC", "XG", Padding.LOW), ), - vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.HIGH),), + vertical_dimensions=(DimDimPadding("ZC", "ZG", Padding.LOW),), ), ) .pipe( diff --git a/src/parcels/_python.py b/src/parcels/_python.py index 02fd6ea79..b7f8b2845 100644 --- a/src/parcels/_python.py +++ b/src/parcels/_python.py @@ -14,7 +14,7 @@ def isinstance_noimport(obj, class_or_tuple): ) -def _repr_from_dunder_dict(obj: object) -> str: +def repr_from_dunder_dict(obj: object) -> str: """Dataclass-like __repr__ implementation based on __dict__.""" parts = [f"{k}={v!r}" for k, v in obj.__dict__.items()] return f"{obj.__class__.__qualname__}(" + ", ".join(parts) + ")" diff --git a/tests/utils/test_sgrid.py b/tests/utils/test_sgrid.py index 056c9c3a9..7356e08e9 100644 --- a/tests/utils/test_sgrid.py +++ b/tests/utils/test_sgrid.py @@ -38,7 +38,7 @@ def dummy_sgrid_2d_ds(grid: sgrid.Grid2DMetadata) -> xr.Dataset: ds = dummy_comodo_3d_ds() # Can't rename dimensions that already exist in the dataset - assume(sgrid._get_unique_dim_names(grid) & set(ds.dims) == set()) + assume(sgrid.get_unique_dim_names(grid) & set(ds.dims) == set()) renamings = {} if grid.vertical_dimensions is None: @@ -63,7 +63,7 @@ def dummy_sgrid_3d_ds(grid: sgrid.Grid3DMetadata) -> xr.Dataset: ds = dummy_comodo_3d_ds() # Can't rename dimensions that already exist in the dataset - assume(sgrid._get_unique_dim_names(grid) & set(ds.dims) == set()) + assume(sgrid.get_unique_dim_names(grid) & set(ds.dims) == set()) renamings = {} for old, new in zip(["XG", "YG", "ZG"], grid.node_dimensions, strict=True): @@ -304,6 +304,7 @@ def test_parse_sgrid_3d(grid_metadata: sgrid.Grid3DMetadata): def test_rename_dims(grids_and_dims_dict): """Creates two SGrid 2D metadata objects with disjoint dimension names, and a mapping between the dimension names. Renames the dimensions of the first grid according to the mapping, and checks that the result + is equal to the second grid. """ grid_old, grid_new, dims_dict = grids_and_dims_dict assert grid_old.rename_dims(dims_dict).to_attrs() == grid_new.to_attrs() From cd2bfcc81ebf33888b181d009c297e88f9a2da04 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:55:42 +0100 Subject: [PATCH 08/13] Fix type annotation --- src/parcels/_python.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parcels/_python.py b/src/parcels/_python.py index b7f8b2845..1cc7b4fd5 100644 --- a/src/parcels/_python.py +++ b/src/parcels/_python.py @@ -1,6 +1,6 @@ # Generic Python helpers import inspect -from collections.abc import Callable +from types import FunctionType def isinstance_noimport(obj, class_or_tuple): @@ -20,7 +20,7 @@ def repr_from_dunder_dict(obj: object) -> str: return f"{obj.__class__.__qualname__}(" + ", ".join(parts) + ")" -def assert_same_function_signature(f: Callable, *, ref: Callable, context: str) -> None: +def assert_same_function_signature(f: FunctionType, *, ref: FunctionType, context: str) -> None: """Ensures a function `f` has the same signature as the reference function `ref`.""" sig_ref = inspect.signature(ref) sig = inspect.signature(f) From 74059fb6b30b94c052321b7ee0983bc064cd27d7 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 19:57:44 +0100 Subject: [PATCH 09/13] Review feedback --- src/parcels/_datasets/structured/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parcels/_datasets/structured/generic.py b/src/parcels/_datasets/structured/generic.py index 9f952cd0e..33ad85185 100644 --- a/src/parcels/_datasets/structured/generic.py +++ b/src/parcels/_datasets/structured/generic.py @@ -248,7 +248,7 @@ def _unrolled_cone_curvilinear_grid(): "2d_left_unrolled_cone": _unrolled_cone_curvilinear_grid(), } -_COMODO_TO_2D_SGRID = { # Note "2D SGRID" here is meant in the context of Grid2DMetadata and SGRID convention (i.e., 1D depth) +_COMODO_TO_2D_SGRID = { # Note "2D SGRID" here is meant in the context of SGRID convention (i.e., 1D depth) "XG": "node_dimension1", "YG": "node_dimension2", "XC": "face_dimension1", From a31a680b7d8ac709be2e05ead3d33da3187bcb16 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Thu, 11 Dec 2025 20:05:37 +0100 Subject: [PATCH 10/13] Fix name --- tests/utils/test_sgrid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/test_sgrid.py b/tests/utils/test_sgrid.py index 7356e08e9..b25fc5125 100644 --- a/tests/utils/test_sgrid.py +++ b/tests/utils/test_sgrid.py @@ -193,7 +193,7 @@ def test_Grid3DMetadata_roundtrip(grid: sgrid.Grid3DMetadata): @given(sgrid_strategies.grid_metadata) -def test_parse_grid_attrs(grid: sgrid.SGridMetadataProtocol): +def test_parse_grid_attrs(grid: sgrid.AttrsSerializable): attrs = grid.to_attrs() parsed = sgrid.parse_grid_attrs(attrs) assert parsed == grid From 436d6063c1ab71273a24440d89803986ae093920 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Fri, 12 Dec 2025 09:09:57 +0100 Subject: [PATCH 11/13] Review feedback --- src/parcels/_core/utils/sgrid.py | 4 ++-- src/parcels/_datasets/structured/generic.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/parcels/_core/utils/sgrid.py b/src/parcels/_core/utils/sgrid.py index d2c8e6b4c..dbcadaef9 100644 --- a/src/parcels/_core/utils/sgrid.py +++ b/src/parcels/_core/utils/sgrid.py @@ -398,7 +398,7 @@ def rename_dims(ds: xr.Dataset, dims_dict: dict[str, str]) -> xr.Dataset: grid_da = get_grid_topology(ds) if grid_da is None: raise ValueError( - "No variable found in dataset with 'cf_role' attribute set to 'grid_topology'. Is this an SGrid dataset?" + "No variable found in dataset with 'cf_role' attribute set to 'grid_topology'. This doesn't look to be an SGrid dataset - please make your dataset conforms to SGrid conventions." ) ds = ds.rename_dims(dims_dict) @@ -417,7 +417,7 @@ def get_unique_dim_names(grid: Grid2DMetadata | Grid3DMetadata) -> set[str]: if key in ("cf_role", "topology_dimension") or value is None: continue assert isinstance(value, tuple), ( - f"Expected sgrid metadata attribute to be represented as a tuple, got {value!r}. Is '{key}' a valid SGrid metadata attribute?" + f"Expected sgrid metadata attribute to be represented as a tuple, got {value!r}. This is an internal error to Parcels - please post an issue if you encounter this." ) for item in value: if isinstance(item, DimDimPadding): diff --git a/src/parcels/_datasets/structured/generic.py b/src/parcels/_datasets/structured/generic.py index 33ad85185..df97b0cf3 100644 --- a/src/parcels/_datasets/structured/generic.py +++ b/src/parcels/_datasets/structured/generic.py @@ -26,7 +26,7 @@ def _attach_sgrid_metadata(ds, grid: Grid2DMetadata | Grid3DMetadata): 0, grid.to_attrs(), ) - ds.attrs["conventions"] = "SGRID" + ds.attrs["Conventions"] = "SGRID" return ds From 997df5c2a672d08a51b3a6561df193a13556f5ee Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Fri, 12 Dec 2025 09:37:10 +0100 Subject: [PATCH 12/13] Update test_rename_dims --- tests/utils/test_sgrid.py | 103 ++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 61 deletions(-) diff --git a/tests/utils/test_sgrid.py b/tests/utils/test_sgrid.py index b25fc5125..094c1c6ca 100644 --- a/tests/utils/test_sgrid.py +++ b/tests/utils/test_sgrid.py @@ -3,7 +3,6 @@ import xarray as xr import xgcm from hypothesis import assume, example, given -from hypothesis import strategies as st from parcels._core.utils import sgrid from tests.strategies import sgrid as sgrid_strategies @@ -238,76 +237,58 @@ def test_parse_sgrid_3d(grid_metadata: sgrid.Grid3DMetadata): assert coords[sgrid.SGRID_PADDING_TO_XGCM_POSITION[padding]] == dim_node -@example( - grids_and_dims_dict=( - sgrid.Grid2DMetadata( - cf_role="grid_topology", - topology_dimension=2, - node_dimensions=("node_dimension1", "node_dimension2"), - face_dimensions=( - sgrid.DimDimPadding("face_dimension1", "node_dimension1", sgrid.Padding.LOW), - sgrid.DimDimPadding("face_dimension2", "node_dimension2", sgrid.Padding.LOW), - ), - vertical_dimensions=( - sgrid.DimDimPadding("vertical_dimensions_dim1", "vertical_dimensions_dim2", sgrid.Padding.LOW), - ), - ), - sgrid.Grid2DMetadata( - cf_role="grid_topology", - topology_dimension=2, - node_dimensions=("new_node_dimension1", "new_node_dimension2"), - face_dimensions=( - sgrid.DimDimPadding("new_face_dimension1", "new_node_dimension1", sgrid.Padding.LOW), - sgrid.DimDimPadding("new_face_dimension2", "new_node_dimension2", sgrid.Padding.LOW), - ), - vertical_dimensions=( - sgrid.DimDimPadding("new_vertical_dimensions_dim1", "new_vertical_dimensions_dim2", sgrid.Padding.LOW), - ), - ), - { - "node_dimension1": "new_node_dimension1", - "node_dimension2": "new_node_dimension2", - "face_dimension1": "new_face_dimension1", - "face_dimension2": "new_face_dimension2", - "vertical_dimensions_dim1": "new_vertical_dimensions_dim1", - "vertical_dimensions_dim2": "new_vertical_dimensions_dim2", - }, - ) -) -@given( - grids_and_dims_dict=st.lists(sgrid_strategies.dimension_name, min_size=12, max_size=12, unique=True).map( - lambda dims: ( +@pytest.mark.parametrize( + "grid", + [ + ( sgrid.Grid2DMetadata( cf_role="grid_topology", topology_dimension=2, - node_dimensions=(dims[0], dims[1]), + node_dimensions=("node_dimension1", "node_dimension2"), face_dimensions=( - sgrid.DimDimPadding(dims[2], dims[0], sgrid.Padding.LOW), - sgrid.DimDimPadding(dims[3], dims[1], sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension1", "node_dimension1", sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension2", "node_dimension2", sgrid.Padding.LOW), + ), + vertical_dimensions=( + sgrid.DimDimPadding("vertical_dimensions_dim1", "vertical_dimensions_dim2", sgrid.Padding.LOW), ), - vertical_dimensions=(sgrid.DimDimPadding(dims[4], dims[5], sgrid.Padding.LOW),), - ), + ) + ), + ( sgrid.Grid2DMetadata( cf_role="grid_topology", topology_dimension=2, - node_dimensions=(dims[6 + 0], dims[6 + 1]), + node_dimensions=("node_dimension1", "node_dimension2"), face_dimensions=( - sgrid.DimDimPadding(dims[6 + 2], dims[6 + 0], sgrid.Padding.LOW), - sgrid.DimDimPadding(dims[6 + 3], dims[6 + 1], sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension1", "node_dimension1", sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension2", "node_dimension2", sgrid.Padding.LOW), ), - vertical_dimensions=(sgrid.DimDimPadding(dims[6 + 4], dims[6 + 5], sgrid.Padding.LOW),), - ), - {k: v for k, v in zip(dims[:6], dims[6:], strict=True)}, - ) - ) + vertical_dimensions=None, + ) + ), + ( + sgrid.Grid3DMetadata( + cf_role="grid_topology", + topology_dimension=3, + node_dimensions=("node_dimension1", "node_dimension2", "node_dimension3"), + volume_dimensions=( + sgrid.DimDimPadding("face_dimension1", "node_dimension1", sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension2", "node_dimension2", sgrid.Padding.LOW), + sgrid.DimDimPadding("face_dimension3", "node_dimension3", sgrid.Padding.LOW), + ), + ) + ), + ], ) -def test_rename_dims(grids_and_dims_dict): - """Creates two SGrid 2D metadata objects with disjoint dimension names, and a mapping between the dimension names. - Renames the dimensions of the first grid according to the mapping, and checks that the result - is equal to the second grid. - """ - grid_old, grid_new, dims_dict = grids_and_dims_dict - assert grid_old.rename_dims(dims_dict).to_attrs() == grid_new.to_attrs() +def test_rename_dims(grid): + dims = sgrid.get_unique_dim_names(grid) + dims_dict = {dim: f"new_{dim}" for dim in dims} + dims_dict_inv = {v: k for k, v in dims_dict.items()} + + grid_new = grid.rename_dims(dims_dict) + assert dims & set(sgrid.get_unique_dim_names(grid_new)) == set() + + assert grid == grid_new.rename_dims(dims_dict_inv) def test_rename_dims_errors(grid2dmetadata): @@ -318,7 +299,7 @@ def test_rename_dims_errors(grid2dmetadata): "node_dimension1": "new_node_dimension", "node_dimension2": "new_node_dimension", } - with pytest.raises(AssertionError, match="dims_dict contains non-unique target dimension names"): + with pytest.raises(AssertionError, match="dims_dict contains duplicate target dimension names"): grid.rename_dims(dims_dict) # Unexpected attribute in dims_dict dims_dict = { From 94a8e1cb49ee70efc8d06bac32a2c8f4e7ec79f3 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Fri, 12 Dec 2025 09:38:08 +0100 Subject: [PATCH 13/13] Update sgrid __eq__ methods --- src/parcels/_core/utils/sgrid.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/parcels/_core/utils/sgrid.py b/src/parcels/_core/utils/sgrid.py index dbcadaef9..bd98962cc 100644 --- a/src/parcels/_core/utils/sgrid.py +++ b/src/parcels/_core/utils/sgrid.py @@ -111,13 +111,7 @@ def __repr__(self) -> str: def __eq__(self, other: Any) -> bool: if not isinstance(other, Grid2DMetadata): return NotImplemented - return ( - self.cf_role == other.cf_role - and self.topology_dimension == other.topology_dimension - and self.node_dimensions == other.node_dimensions - and self.face_dimensions == other.face_dimensions - and self.vertical_dimensions == other.vertical_dimensions - ) + return self.to_attrs() == other.to_attrs() @classmethod def from_attrs(cls, attrs): @@ -203,12 +197,7 @@ def __repr__(self) -> str: def __eq__(self, other: Any) -> bool: if not isinstance(other, Grid3DMetadata): return NotImplemented - return ( - self.cf_role == other.cf_role - and self.topology_dimension == other.topology_dimension - and self.node_dimensions == other.node_dimensions - and self.volume_dimensions == other.volume_dimensions - ) + return self.to_attrs() == other.to_attrs() @classmethod def from_attrs(cls, attrs):