Skip to content

Commit 8e5e3da

Browse files
Remove interpolation function annotation checking and refactor (#2077)
* Move code * Update vector interp template * Update interp function validation to asserts * 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 * Refactor interpolation function checking * fix test * Add default interpolator mapping
1 parent 73e0487 commit 8e5e3da

2 files changed

Lines changed: 85 additions & 85 deletions

File tree

parcels/field.py

Lines changed: 54 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,45 @@ def _deal_with_errors(error, key, vector_type: VectorType):
5959
return 0
6060

6161

62+
def ZeroInterpolator(
63+
field: Field,
64+
ti: int,
65+
position: dict[str, tuple[int, float | np.ndarray]],
66+
tau: np.float32 | np.float64,
67+
t: np.float32 | np.float64,
68+
z: np.float32 | np.float64,
69+
y: np.float32 | np.float64,
70+
x: np.float32 | np.float64,
71+
) -> np.float32 | np.float64:
72+
"""Template function used for the signature check of the lateral interpolation methods."""
73+
return 0.0
74+
75+
76+
_DEFAULT_INTERPOLATOR_MAPPING = {
77+
XGrid: ZeroInterpolator, # TODO v4: Update these to better defaults
78+
UxGrid: ZeroInterpolator,
79+
}
80+
81+
82+
def _assert_same_function_signature(f: Callable, *, ref: Callable) -> None:
83+
"""Ensures a function `f` has the same signature as the reference function `ref`."""
84+
sig_ref = inspect.signature(ref)
85+
sig = inspect.signature(f)
86+
87+
if len(sig_ref.parameters) != len(sig.parameters):
88+
raise ValueError(
89+
f"Interpolation function must have {len(sig_ref.parameters)} parameters, got {len(sig.parameters)}"
90+
)
91+
92+
for (_name1, param1), (_name2, param2) in zip(sig_ref.parameters.items(), sig.parameters.items(), strict=False):
93+
if param1.kind != param2.kind:
94+
raise ValueError(
95+
f"Parameter '{_name2}' has incorrect parameter kind. Expected {param1.kind}, got {param2.kind}"
96+
)
97+
if param1.name != param2.name:
98+
raise ValueError(f"Parameter '{_name2}' has incorrect name. Expected '{param1.name}', got '{param2.name}'")
99+
100+
62101
class Field:
63102
"""The Field class that holds scalar field data.
64103
The `Field` object is a wrapper around a xarray.DataArray or uxarray.UxDataArray object.
@@ -93,44 +132,6 @@ class Field:
93132
94133
"""
95134

96-
@staticmethod
97-
def _interp_template(
98-
self,
99-
ti: int,
100-
position: dict[str, tuple[int, float | np.ndarray]],
101-
tau: np.float32 | np.float64,
102-
t: np.float32 | np.float64,
103-
z: np.float32 | np.float64,
104-
y: np.float32 | np.float64,
105-
x: np.float32 | np.float64,
106-
) -> np.float32 | np.float64:
107-
"""Template function used for the signature check of the lateral interpolation methods."""
108-
return 0.0
109-
110-
def _validate_interp_function(self, func: Callable) -> bool:
111-
"""Ensures that the function has the correct signature."""
112-
template_sig = inspect.signature(self._interp_template)
113-
func_sig = inspect.signature(func)
114-
115-
if len(template_sig.parameters) != len(func_sig.parameters):
116-
return False
117-
118-
for (_name1, param1), (_name2, param2) in zip(
119-
template_sig.parameters.items(), func_sig.parameters.items(), strict=False
120-
):
121-
if param1.kind != param2.kind:
122-
return False
123-
if param1.annotation != param2.annotation:
124-
return False
125-
126-
return_annotation = func_sig.return_annotation
127-
template_return = template_sig.return_annotation
128-
129-
if return_annotation != template_return:
130-
return False
131-
132-
return True
133-
134135
def __init__(
135136
self,
136137
name: str,
@@ -178,9 +179,9 @@ def __init__(
178179

179180
# Setting the interpolation method dynamically
180181
if interp_method is None:
181-
self._interp_method = self._interp_template # Default to method that returns 0 always
182+
self._interp_method = _DEFAULT_INTERPOLATOR_MAPPING[type(self.grid)]
182183
else:
183-
self._validate_interp_function(interp_method)
184+
_assert_same_function_signature(interp_method, ref=ZeroInterpolator)
184185
self._interp_method = interp_method
185186

186187
self.igrid = -1 # Default the grid index to -1
@@ -272,7 +273,7 @@ def interp_method(self):
272273

273274
@interp_method.setter
274275
def interp_method(self, method: Callable):
275-
self._validate_interp_function(method)
276+
_assert_same_function_signature(method, ref=ZeroInterpolator)
276277
self._interp_method = method
277278

278279
def _check_velocitysampling(self):
@@ -283,16 +284,6 @@ def _check_velocitysampling(self):
283284
stacklevel=2,
284285
)
285286

286-
def __getitem__(self, key):
287-
self._check_velocitysampling()
288-
try:
289-
if _isParticle(key):
290-
return self.eval(key.time, key.depth, key.lat, key.lon, key)
291-
else:
292-
return self.eval(*key)
293-
except tuple(AllParcelsErrorCodes.keys()) as error:
294-
return _deal_with_errors(error, key, vector_type=None)
295-
296287
def eval(self, time: datetime, z, y, x, particle=None, applyConversion=True):
297288
"""Interpolate field values in space and time.
298289
@@ -332,44 +323,23 @@ def _rescale_and_set_minmax(self, data):
332323
def __getattr__(self, key: str):
333324
return getattr(self.data, key)
334325

326+
def __getitem__(self, key):
327+
self._check_velocitysampling()
328+
try:
329+
if _isParticle(key):
330+
return self.eval(key.time, key.depth, key.lat, key.lon, key)
331+
else:
332+
return self.eval(*key)
333+
except tuple(AllParcelsErrorCodes.keys()) as error:
334+
return _deal_with_errors(error, key, vector_type=None)
335+
335336
def __contains__(self, key: str):
336337
return key in self.data
337338

338339

339340
class VectorField:
340341
"""VectorField class that holds vector field data needed to execute particles."""
341342

342-
@staticmethod
343-
def _vector_interp_template(
344-
self,
345-
ti: int,
346-
ei: int,
347-
bcoords: np.ndarray,
348-
t: np.float32 | np.float64,
349-
z: np.float32 | np.float64,
350-
y: np.float32 | np.float64,
351-
x: np.float32 | np.float64,
352-
) -> np.float32 | np.float64:
353-
"""Template function used for the signature check of the lateral interpolation methods."""
354-
return 0.0
355-
356-
def _validate_vector_interp_function(self, func: Callable):
357-
"""Ensures that the function has the correct signature."""
358-
expected_params = ["ti", "ei", "bcoords", "t", "z", "y", "x"]
359-
expected_return_types = (np.float32, np.float64)
360-
361-
sig = inspect.signature(func)
362-
params = list(sig.parameters.keys())
363-
364-
# Check the parameter names and count
365-
if params != expected_params:
366-
raise TypeError(f"Function must have parameters {expected_params}, but got {params}")
367-
368-
# Check return annotation if present
369-
return_annotation = sig.return_annotation
370-
if return_annotation not in (inspect.Signature.empty, *expected_return_types):
371-
raise TypeError(f"Function must return a float, but got {return_annotation}")
372-
373343
def __init__(
374344
self, name: str, U: Field, V: Field, W: Field | None = None, vector_interp_method: Callable | None = None
375345
):
@@ -395,7 +365,7 @@ def __init__(
395365
if vector_interp_method is None:
396366
self._vector_interp_method = None
397367
else:
398-
self._validate_vector_interp_function(vector_interp_method)
368+
_assert_same_function_signature(vector_interp_method, ref=ZeroInterpolator)
399369
self._interp_method = vector_interp_method
400370

401371
def __repr__(self):
@@ -411,7 +381,7 @@ def vector_interp_method(self):
411381

412382
@vector_interp_method.setter
413383
def vector_interp_method(self, method: Callable):
414-
self._validate_vector_interp_function(method)
384+
_assert_same_function_signature(method, ref=ZeroInterpolator)
415385
self._vector_interp_method = method
416386

417387
# @staticmethod

tests/v4/test_field.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
from __future__ import annotations
2+
13
import numpy as np
24
import pytest
35
import uxarray as ux
46
import xarray as xr
57

6-
from parcels import Field, UXPiecewiseConstantFace, UXPiecewiseLinearNode, xgcm
8+
from parcels import Field, UXPiecewiseConstantFace, UXPiecewiseLinearNode, VectorField, xgcm
79
from parcels._datasets.structured.generic import T as T_structured
810
from parcels._datasets.structured.generic import datasets as datasets_structured
911
from parcels._datasets.unstructured.generic import datasets as datasets_unstructured
@@ -115,6 +117,34 @@ def test_vectorfield_init_different_time_intervals():
115117
...
116118

117119

120+
def test_field_invalid_interpolator():
121+
ds = datasets_structured["ds_2d_left"]
122+
grid = XGrid(xgcm.Grid(ds))
123+
124+
def invalid_interpolator_wrong_signature(self, ti, position, tau, t, z, y, invalid):
125+
return 0.0
126+
127+
# Test invalid interpolator with wrong signature
128+
with pytest.raises(ValueError, match=".*incorrect name.*"):
129+
Field(name="test", data=ds["data_g"], grid=grid, interp_method=invalid_interpolator_wrong_signature)
130+
131+
132+
def test_vectorfield_invalid_interpolator():
133+
ds = datasets_structured["ds_2d_left"]
134+
grid = XGrid(xgcm.Grid(ds))
135+
136+
def invalid_interpolator_wrong_signature(self, ti, position, tau, t, z, y, invalid):
137+
return 0.0
138+
139+
# Create component fields
140+
U = Field(name="U", data=ds["data_g"], grid=grid)
141+
V = Field(name="V", data=ds["data_g"], grid=grid)
142+
143+
# Test invalid interpolator with wrong signature
144+
with pytest.raises(ValueError, match=".*incorrect name.*"):
145+
VectorField(name="UV", U=U, V=V, vector_interp_method=invalid_interpolator_wrong_signature)
146+
147+
118148
def test_field_unstructured_z_linear():
119149
"""Tests correctness of piecewise constant and piecewise linear interpolation methods on an unstructured grid with a vertical coordinate.
120150
The example dataset is a FESOM2 square Delaunay grid with uniform z-coordinate. Cell centered and layer registered data are defined to be

0 commit comments

Comments
 (0)