From d55bdfba462a6549ba2393ec2caa5cfd3108775b Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 12:55:00 +0200 Subject: [PATCH 1/7] Move code --- parcels/field.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/parcels/field.py b/parcels/field.py index 473ecbc833..0d98b7b95b 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -283,16 +283,6 @@ def _check_velocitysampling(self): stacklevel=2, ) - def __getitem__(self, key): - self._check_velocitysampling() - try: - if _isParticle(key): - return self.eval(key.time, key.depth, key.lat, key.lon, key) - else: - return self.eval(*key) - except tuple(AllParcelsErrorCodes.keys()) as error: - return _deal_with_errors(error, key, vector_type=None) - def eval(self, time: datetime, z, y, x, particle=None, applyConversion=True): """Interpolate field values in space and time. @@ -332,6 +322,16 @@ def _rescale_and_set_minmax(self, data): def __getattr__(self, key: str): return getattr(self.data, key) + def __getitem__(self, key): + self._check_velocitysampling() + try: + if _isParticle(key): + return self.eval(key.time, key.depth, key.lat, key.lon, key) + else: + return self.eval(*key) + except tuple(AllParcelsErrorCodes.keys()) as error: + return _deal_with_errors(error, key, vector_type=None) + def __contains__(self, key: str): return key in self.data From 559f37a11acadb141941e6e6b4f77f969357cf09 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 13:05:18 +0200 Subject: [PATCH 2/7] Update vector interp template --- parcels/field.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parcels/field.py b/parcels/field.py index 0d98b7b95b..a4fa54d5d9 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -343,8 +343,8 @@ class VectorField: def _vector_interp_template( self, ti: int, - ei: int, - bcoords: np.ndarray, + position: dict[str, tuple[int, float | np.ndarray]], + tau: np.float32 | np.float64, t: np.float32 | np.float64, z: np.float32 | np.float64, y: np.float32 | np.float64, From 5cd8f84d4bc67be9d39f90855c3a28dc9319541a Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 15:33:28 +0200 Subject: [PATCH 3/7] Update interp function validation to asserts --- parcels/field.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/parcels/field.py b/parcels/field.py index a4fa54d5d9..6b37a79afa 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -107,29 +107,35 @@ def _interp_template( """Template function used for the signature check of the lateral interpolation methods.""" return 0.0 - def _validate_interp_function(self, func: Callable) -> bool: + def _validate_interp_function(self, func: Callable) -> None: """Ensures that the function has the correct signature.""" template_sig = inspect.signature(self._interp_template) func_sig = inspect.signature(func) if len(template_sig.parameters) != len(func_sig.parameters): - return False + raise ValueError( + f"Interpolation function must have {len(template_sig.parameters)} parameters, got {len(func_sig.parameters)}" + ) for (_name1, param1), (_name2, param2) in zip( template_sig.parameters.items(), func_sig.parameters.items(), strict=False ): if param1.kind != param2.kind: - return False + raise ValueError( + f"Parameter '{_name2}' has incorrect parameter kind. Expected {param1.kind}, got {param2.kind}" + ) if param1.annotation != param2.annotation: - return False + raise ValueError( + f"Parameter '{_name2}' has incorrect type annotation. Expected {param1.annotation}, got {param2.annotation}" + ) return_annotation = func_sig.return_annotation template_return = template_sig.return_annotation if return_annotation != template_return: - return False - - return True + raise ValueError( + f"Interpolation function has incorrect return type. Expected {template_return}, got {return_annotation}" + ) def __init__( self, From e31a2f72df7c5a5e17624cae13cfbb19f1ab7688 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 15:40:05 +0200 Subject: [PATCH 4/7] Test interpolation function checking (and remove checking of type annotations) Dynamically checking of type annotations like this is flaky due to how Python internally handles type annotations --- parcels/field.py | 26 ++------------------------ tests/v4/test_field.py | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/parcels/field.py b/parcels/field.py index 6b37a79afa..106bb1227f 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -124,19 +124,11 @@ def _validate_interp_function(self, func: Callable) -> None: raise ValueError( f"Parameter '{_name2}' has incorrect parameter kind. Expected {param1.kind}, got {param2.kind}" ) - if param1.annotation != param2.annotation: + if param1.name != param2.name: raise ValueError( - f"Parameter '{_name2}' has incorrect type annotation. Expected {param1.annotation}, got {param2.annotation}" + f"Parameter '{_name2}' has incorrect name. Expected '{param1.name}', got '{param2.name}'" ) - return_annotation = func_sig.return_annotation - template_return = template_sig.return_annotation - - if return_annotation != template_return: - raise ValueError( - f"Interpolation function has incorrect return type. Expected {template_return}, got {return_annotation}" - ) - def __init__( self, name: str, @@ -345,20 +337,6 @@ def __contains__(self, key: str): class VectorField: """VectorField class that holds vector field data needed to execute particles.""" - @staticmethod - def _vector_interp_template( - self, - ti: int, - position: dict[str, tuple[int, float | np.ndarray]], - tau: np.float32 | np.float64, - t: np.float32 | np.float64, - z: np.float32 | np.float64, - y: np.float32 | np.float64, - x: np.float32 | np.float64, - ) -> np.float32 | np.float64: - """Template function used for the signature check of the lateral interpolation methods.""" - return 0.0 - def _validate_vector_interp_function(self, func: Callable): """Ensures that the function has the correct signature.""" expected_params = ["ti", "ei", "bcoords", "t", "z", "y", "x"] diff --git a/tests/v4/test_field.py b/tests/v4/test_field.py index b336129dfe..0f01f9e550 100644 --- a/tests/v4/test_field.py +++ b/tests/v4/test_field.py @@ -1,9 +1,11 @@ +from __future__ import annotations + import numpy as np import pytest import uxarray as ux import xarray as xr -from parcels import Field, UXPiecewiseConstantFace, UXPiecewiseLinearNode, xgcm +from parcels import Field, UXPiecewiseConstantFace, UXPiecewiseLinearNode, VectorField, xgcm from parcels._datasets.structured.generic import T as T_structured from parcels._datasets.structured.generic import datasets as datasets_structured from parcels._datasets.unstructured.generic import datasets as datasets_unstructured @@ -115,6 +117,38 @@ def test_vectorfield_init_different_time_intervals(): ... +def test_field_invalid_interpolator(): + """Test that Field initialization fails with invalid interpolation methods.""" + ds = datasets_structured["ds_2d_left"] + grid = XGrid(xgcm.Grid(ds)) + + def invalid_interpolator_wrong_signature(self, ti, position, tau, t, z, y, invalid): + return 0.0 + + # Test invalid interpolator with wrong signature + with pytest.raises(ValueError, match=".*incorrect name.*"): + Field(name="test", data=ds["data_g"], grid=grid, interp_method=invalid_interpolator_wrong_signature) + + +@pytest.mark.xfail +def test_vectorfield_invalid_interpolator(): + """Test that VectorField initialization fails with invalid interpolation methods.""" + ds = datasets_structured["ds_2d_left"] + grid = XGrid(xgcm.Grid(ds)) + + def invalid_interpolator_wrong_signature(self): + # Missing required parameters from _interp_template signature + return 0.0 + + # Create component fields + U = Field(name="U", data=ds["data_g"], grid=grid) + V = Field(name="V", data=ds["data_g"], grid=grid) + + # Test invalid interpolator with wrong signature + with pytest.raises(ValueError, match=".*incorrect name.*"): + VectorField(name="UV", U=U, V=V, vector_interp_method=invalid_interpolator_wrong_signature) + + def test_field_unstructured_z_linear(): """Tests correctness of piecewise constant and piecewise linear interpolation methods on an unstructured grid with a vertical coordinate. The example dataset is a FESOM2 square Delaunay grid with uniform z-coordinate. Cell centered and layer registered data are defined to be From fbeb39dcfad6e50403940561134f45ed4d691fa5 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 16:30:56 +0200 Subject: [PATCH 5/7] Refactor interpolation function checking --- parcels/field.py | 96 +++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 58 deletions(-) diff --git a/parcels/field.py b/parcels/field.py index 106bb1227f..785c7bf344 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -59,6 +59,39 @@ def _deal_with_errors(error, key, vector_type: VectorType): return 0 +def ZeroInterpolator( + field: Field, + ti: int, + position: dict[str, tuple[int, float | np.ndarray]], + tau: np.float32 | np.float64, + t: np.float32 | np.float64, + z: np.float32 | np.float64, + y: np.float32 | np.float64, + x: np.float32 | np.float64, +) -> np.float32 | np.float64: + """Template function used for the signature check of the lateral interpolation methods.""" + return 0.0 + + +def _assert_same_function_signature(f: Callable, *, ref: Callable) -> None: + """Ensures a function `f` has the same signature as the reference function `ref`.""" + sig_ref = inspect.signature(ref) + sig = inspect.signature(f) + + if len(sig_ref.parameters) != len(sig.parameters): + raise ValueError( + f"Interpolation function must have {len(sig_ref.parameters)} parameters, got {len(sig.parameters)}" + ) + + for (_name1, param1), (_name2, param2) in zip(sig_ref.parameters.items(), sig.parameters.items(), strict=False): + if param1.kind != param2.kind: + raise ValueError( + f"Parameter '{_name2}' has incorrect parameter kind. Expected {param1.kind}, got {param2.kind}" + ) + if param1.name != param2.name: + raise ValueError(f"Parameter '{_name2}' has incorrect name. Expected '{param1.name}', got '{param2.name}'") + + class Field: """The Field class that holds scalar field data. The `Field` object is a wrapper around a xarray.DataArray or uxarray.UxDataArray object. @@ -93,42 +126,6 @@ class Field: """ - @staticmethod - def _interp_template( - self, - ti: int, - position: dict[str, tuple[int, float | np.ndarray]], - tau: np.float32 | np.float64, - t: np.float32 | np.float64, - z: np.float32 | np.float64, - y: np.float32 | np.float64, - x: np.float32 | np.float64, - ) -> np.float32 | np.float64: - """Template function used for the signature check of the lateral interpolation methods.""" - return 0.0 - - def _validate_interp_function(self, func: Callable) -> None: - """Ensures that the function has the correct signature.""" - template_sig = inspect.signature(self._interp_template) - func_sig = inspect.signature(func) - - if len(template_sig.parameters) != len(func_sig.parameters): - raise ValueError( - f"Interpolation function must have {len(template_sig.parameters)} parameters, got {len(func_sig.parameters)}" - ) - - for (_name1, param1), (_name2, param2) in zip( - template_sig.parameters.items(), func_sig.parameters.items(), strict=False - ): - if param1.kind != param2.kind: - raise ValueError( - f"Parameter '{_name2}' has incorrect parameter kind. Expected {param1.kind}, got {param2.kind}" - ) - if param1.name != param2.name: - raise ValueError( - f"Parameter '{_name2}' has incorrect name. Expected '{param1.name}', got '{param2.name}'" - ) - def __init__( self, name: str, @@ -176,9 +173,9 @@ def __init__( # Setting the interpolation method dynamically if interp_method is None: - self._interp_method = self._interp_template # Default to method that returns 0 always + self._interp_method = ZeroInterpolator # Default to method that returns 0 always else: - self._validate_interp_function(interp_method) + _assert_same_function_signature(interp_method, ref=ZeroInterpolator) self._interp_method = interp_method self.igrid = -1 # Default the grid index to -1 @@ -270,7 +267,7 @@ def interp_method(self): @interp_method.setter def interp_method(self, method: Callable): - self._validate_interp_function(method) + _assert_same_function_signature(method, ref=ZeroInterpolator) self._interp_method = method def _check_velocitysampling(self): @@ -337,23 +334,6 @@ def __contains__(self, key: str): class VectorField: """VectorField class that holds vector field data needed to execute particles.""" - def _validate_vector_interp_function(self, func: Callable): - """Ensures that the function has the correct signature.""" - expected_params = ["ti", "ei", "bcoords", "t", "z", "y", "x"] - expected_return_types = (np.float32, np.float64) - - sig = inspect.signature(func) - params = list(sig.parameters.keys()) - - # Check the parameter names and count - if params != expected_params: - raise TypeError(f"Function must have parameters {expected_params}, but got {params}") - - # Check return annotation if present - return_annotation = sig.return_annotation - if return_annotation not in (inspect.Signature.empty, *expected_return_types): - raise TypeError(f"Function must return a float, but got {return_annotation}") - def __init__( self, name: str, U: Field, V: Field, W: Field | None = None, vector_interp_method: Callable | None = None ): @@ -379,7 +359,7 @@ def __init__( if vector_interp_method is None: self._vector_interp_method = None else: - self._validate_vector_interp_function(vector_interp_method) + _assert_same_function_signature(vector_interp_method, ref=ZeroInterpolator) self._interp_method = vector_interp_method def __repr__(self): @@ -395,7 +375,7 @@ def vector_interp_method(self): @vector_interp_method.setter def vector_interp_method(self, method: Callable): - self._validate_vector_interp_function(method) + _assert_same_function_signature(method, ref=ZeroInterpolator) self._vector_interp_method = method # @staticmethod From 39e949ba4ce0f7fc2765797b64f6269698c7689a Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 16:49:53 +0200 Subject: [PATCH 6/7] fix test --- tests/v4/test_field.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/v4/test_field.py b/tests/v4/test_field.py index 0f01f9e550..a555a672f2 100644 --- a/tests/v4/test_field.py +++ b/tests/v4/test_field.py @@ -118,7 +118,6 @@ def test_vectorfield_init_different_time_intervals(): def test_field_invalid_interpolator(): - """Test that Field initialization fails with invalid interpolation methods.""" ds = datasets_structured["ds_2d_left"] grid = XGrid(xgcm.Grid(ds)) @@ -130,14 +129,11 @@ def invalid_interpolator_wrong_signature(self, ti, position, tau, t, z, y, inval Field(name="test", data=ds["data_g"], grid=grid, interp_method=invalid_interpolator_wrong_signature) -@pytest.mark.xfail def test_vectorfield_invalid_interpolator(): - """Test that VectorField initialization fails with invalid interpolation methods.""" ds = datasets_structured["ds_2d_left"] grid = XGrid(xgcm.Grid(ds)) - def invalid_interpolator_wrong_signature(self): - # Missing required parameters from _interp_template signature + def invalid_interpolator_wrong_signature(self, ti, position, tau, t, z, y, invalid): return 0.0 # Create component fields From 4552cd5e4012f555e797fba72f0a629ff07302b9 Mon Sep 17 00:00:00 2001 From: Vecko <36369090+VeckoTheGecko@users.noreply.github.com> Date: Wed, 9 Jul 2025 16:54:07 +0200 Subject: [PATCH 7/7] Add default interpolator mapping --- parcels/field.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/parcels/field.py b/parcels/field.py index 785c7bf344..3268f11c4e 100644 --- a/parcels/field.py +++ b/parcels/field.py @@ -73,6 +73,12 @@ def ZeroInterpolator( return 0.0 +_DEFAULT_INTERPOLATOR_MAPPING = { + XGrid: ZeroInterpolator, # TODO v4: Update these to better defaults + UxGrid: ZeroInterpolator, +} + + def _assert_same_function_signature(f: Callable, *, ref: Callable) -> None: """Ensures a function `f` has the same signature as the reference function `ref`.""" sig_ref = inspect.signature(ref) @@ -173,7 +179,7 @@ def __init__( # Setting the interpolation method dynamically if interp_method is None: - self._interp_method = ZeroInterpolator # Default to method that returns 0 always + self._interp_method = _DEFAULT_INTERPOLATOR_MAPPING[type(self.grid)] else: _assert_same_function_signature(interp_method, ref=ZeroInterpolator) self._interp_method = interp_method