From f60d76909bb7a7329207c1a423080f5ad8832ff0 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:14:31 +0200 Subject: [PATCH 1/3] small refactor: consolidate spacetime_buffer_size and limit_spec dicts for clarity --- src/virtualship/instruments/adcp.py | 9 ++++---- src/virtualship/instruments/argo_float.py | 9 +++----- src/virtualship/instruments/base.py | 23 ++++++------------- src/virtualship/instruments/ctd.py | 9 ++++---- src/virtualship/instruments/drifter.py | 15 ++++++------ .../instruments/ship_underwater_st.py | 9 +++----- src/virtualship/instruments/xbt.py | 9 ++++---- 7 files changed, 32 insertions(+), 51 deletions(-) diff --git a/src/virtualship/instruments/adcp.py b/src/virtualship/instruments/adcp.py index 49a08120..492b5a8f 100644 --- a/src/virtualship/instruments/adcp.py +++ b/src/virtualship/instruments/adcp.py @@ -61,9 +61,9 @@ class ADCPInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize ADCPInstrument.""" variables = expedition.instruments_config.adcp_config.active_variables() - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer + fetch_spec = { + "spatial": True, + } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -71,8 +71,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=None, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/argo_float.py b/src/virtualship/instruments/argo_float.py index b18c91ba..0d88c1db 100644 --- a/src/virtualship/instruments/argo_float.py +++ b/src/virtualship/instruments/argo_float.py @@ -253,14 +253,12 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for argo float simulation; sensor variables come from config - spacetime_buffer_size = { + fetch_spec = { + "spatial": True, "latlon": 3.0, # [degrees] "time": expedition.instruments_config.argo_float_config.lifetime.total_seconds() / (24 * 3600), # [days] } - limit_spec = { - "spatial": True, # spatial limits; lat/lon constrained to waypoint locations + buffer - } super().__init__( expedition, @@ -268,8 +266,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=False, verbose_progress=True, - spacetime_buffer_size=spacetime_buffer_size, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index 1595f5e4..8012cd35 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -50,8 +50,7 @@ def __init__( allow_time_extrapolation: bool, verbose_progress: bool, from_data: Path | None, - spacetime_buffer_size: dict | None = None, - limit_spec: dict | None = None, + fetch_spec: dict | None = None, ): """Initialise instrument.""" self.expedition = expedition @@ -67,8 +66,7 @@ def __init__( self.add_bathymetry = add_bathymetry self.allow_time_extrapolation = allow_time_extrapolation self.verbose_progress = verbose_progress - self.spacetime_buffer_size = spacetime_buffer_size - self.limit_spec = limit_spec + self.fetch_spec = fetch_spec wp_lats, wp_lons = _get_waypoint_latlons(expedition.schedule.waypoints) wp_times = [ @@ -153,11 +151,11 @@ def _get_copernicus_ds( ) latlon_buffer = self._get_spec_value( - "buffer", "latlon", 0.25 + "latlon", 0.25 ) # [degrees]; default 0.25 deg buffer to ensure coverage in field cell edge cases - depth_min = self._get_spec_value("limit", "depth_min", None) - depth_max = self._get_spec_value("limit", "depth_max", None) - spatial_constraint = self._get_spec_value("limit", "spatial", True) + depth_min = self._get_spec_value("depth_min", None) + depth_max = self._get_spec_value("depth_max", None) + spatial_constraint = self._get_spec_value("spatial", True) min_lon_bound = self.min_lon - latlon_buffer if spatial_constraint else None max_lon_bound = self.max_lon + latlon_buffer if spatial_constraint else None @@ -176,8 +174,6 @@ def _get_copernicus_ds( minimum_depth=depth_min, maximum_depth=depth_max, coordinates_selection_method="outside", - service="arco-geo-series", - chunk_size_limit=1, vertical_axis="elevation", ) @@ -190,7 +186,7 @@ def _generate_fieldset(self) -> parcels.FieldSet: fieldsets_list = [] keys = list(self.variables.keys()) - time_buffer = self._get_spec_value("buffer", "time", 0.0) + time_buffer = self._get_spec_value("time", 0.0) for key in keys: var = self.variables[key] @@ -251,8 +247,3 @@ def _generate_fieldset(self) -> parcels.FieldSet: base_fieldset.add_field(uv) return base_fieldset - - def _get_spec_value(self, spec_type: str, key: str, default=None): - """Helper to extract a value from spacetime_buffer_size or limit_spec.""" - spec = self.spacetime_buffer_size if spec_type == "buffer" else self.limit_spec - return spec.get(key) if spec and spec.get(key) is not None else default diff --git a/src/virtualship/instruments/ctd.py b/src/virtualship/instruments/ctd.py index 48164ca7..6f0eca3f 100644 --- a/src/virtualship/instruments/ctd.py +++ b/src/virtualship/instruments/ctd.py @@ -142,9 +142,9 @@ class CTDInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize CTDInstrument.""" variables = expedition.instruments_config.ctd_config.active_variables() - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer + fetch_spec = { + "spatial": True, + } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -152,8 +152,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=None, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index a771b246..f2d2351c 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -88,13 +88,13 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for drifter simulation; sensor variables come from config - spacetime_buffer_size = { - "latlon": None, + fetch_spec = { + "spatial": True, + "latlon": 30.0, # [degrees]; TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) "time": expedition.instruments_config.drifter_config.lifetime.total_seconds() - / (24 * 3600), # [days] - } - limit_spec = { - "spatial": False, # no spatial limits; generate global fieldset + / ( + 24 * 3600 + ), # [days]; TODO: as above, can potentially be removed in the future "depth_min": abs( expedition.instruments_config.drifter_config.depth_meter ), # [meters] @@ -109,8 +109,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=False, verbose_progress=True, - spacetime_buffer_size=spacetime_buffer_size, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/ship_underwater_st.py b/src/virtualship/instruments/ship_underwater_st.py index 302d991f..1563668a 100644 --- a/src/virtualship/instruments/ship_underwater_st.py +++ b/src/virtualship/instruments/ship_underwater_st.py @@ -67,13 +67,11 @@ def __init__(self, expedition, from_data): variables = ( expedition.instruments_config.ship_underwater_st_config.active_variables() ) - spacetime_buffer_size = { + fetch_spec = { + "spatial": True, "latlon": 0.25, # [degrees] "time": 0.0, # [days] } - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -81,8 +79,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=spacetime_buffer_size, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) diff --git a/src/virtualship/instruments/xbt.py b/src/virtualship/instruments/xbt.py index 92f6b3f2..9b0d6432 100644 --- a/src/virtualship/instruments/xbt.py +++ b/src/virtualship/instruments/xbt.py @@ -95,9 +95,9 @@ class XBTInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize XBTInstrument.""" variables = expedition.instruments_config.xbt_config.active_variables() - limit_spec = { - "spatial": True - } # spatial limits; lat/lon constrained to waypoint locations + buffer + fetch_spec = { + "spatial": True, + } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -105,8 +105,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - spacetime_buffer_size=None, - limit_spec=limit_spec, + fetch_spec=fetch_spec, from_data=from_data, ) From e0187b13db2be3ef5e4e40168350a65fc5090650 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Tue, 21 Jul 2026 15:28:38 +0200 Subject: [PATCH 2/3] to FetchSpec dataclass --- src/virtualship/instruments/adcp.py | 7 ++--- src/virtualship/instruments/argo_float.py | 11 ++++---- src/virtualship/instruments/base.py | 28 +++++++++++++------ src/virtualship/instruments/ctd.py | 7 ++--- src/virtualship/instruments/drifter.py | 19 ++++++------- .../instruments/ship_underwater_st.py | 9 ++---- src/virtualship/instruments/xbt.py | 7 ++--- 7 files changed, 40 insertions(+), 48 deletions(-) diff --git a/src/virtualship/instruments/adcp.py b/src/virtualship/instruments/adcp.py index 492b5a8f..26c8122d 100644 --- a/src/virtualship/instruments/adcp.py +++ b/src/virtualship/instruments/adcp.py @@ -5,7 +5,7 @@ import numpy as np from parcels import ParticleFile, ParticleSet -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.utils import build_particle_class_from_sensors, register_instrument @@ -61,9 +61,6 @@ class ADCPInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize ADCPInstrument.""" variables = expedition.instruments_config.adcp_config.active_variables() - fetch_spec = { - "spatial": True, - } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -71,7 +68,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) diff --git a/src/virtualship/instruments/argo_float.py b/src/virtualship/instruments/argo_float.py index 0d88c1db..96b8e4f0 100644 --- a/src/virtualship/instruments/argo_float.py +++ b/src/virtualship/instruments/argo_float.py @@ -7,7 +7,7 @@ from parcels import ParticleFile, ParticleSet, StatusCode, Variable from parcels.kernels import AdvectionRK2 -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.spacetime import Spacetime @@ -253,12 +253,11 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for argo float simulation; sensor variables come from config - fetch_spec = { - "spatial": True, - "latlon": 3.0, # [degrees] - "time": expedition.instruments_config.argo_float_config.lifetime.total_seconds() + fetch_spec = FetchSpec( + latlon_buffer=3.0, # [degrees] + time_buffer=expedition.instruments_config.argo_float_config.lifetime.total_seconds() / (24 * 3600), # [days] - } + ) super().__init__( expedition, diff --git a/src/virtualship/instruments/base.py b/src/virtualship/instruments/base.py index 8012cd35..f07bc0cf 100644 --- a/src/virtualship/instruments/base.py +++ b/src/virtualship/instruments/base.py @@ -2,6 +2,7 @@ import abc import collections +from dataclasses import dataclass from datetime import timedelta from itertools import pairwise from pathlib import Path @@ -28,6 +29,17 @@ from virtualship.models import Expedition +@dataclass +class FetchSpec: + """Fetch constraints and parameters for dataset retrieval.""" + + spatial: bool = True + latlon_buffer: float = 0.25 # degrees + time_buffer: float = 0.0 # days + depth_min: float | None = None + depth_max: float | None = None + + class Instrument(abc.ABC): """Base class for instruments and their simulation.""" @@ -50,7 +62,7 @@ def __init__( allow_time_extrapolation: bool, verbose_progress: bool, from_data: Path | None, - fetch_spec: dict | None = None, + fetch_spec: FetchSpec | None = None, ): """Initialise instrument.""" self.expedition = expedition @@ -66,7 +78,7 @@ def __init__( self.add_bathymetry = add_bathymetry self.allow_time_extrapolation = allow_time_extrapolation self.verbose_progress = verbose_progress - self.fetch_spec = fetch_spec + self.fetch_spec = fetch_spec or FetchSpec() wp_lats, wp_lons = _get_waypoint_latlons(expedition.schedule.waypoints) wp_times = [ @@ -150,12 +162,10 @@ def _get_copernicus_ds( variable=var if not physical else None, ) - latlon_buffer = self._get_spec_value( - "latlon", 0.25 - ) # [degrees]; default 0.25 deg buffer to ensure coverage in field cell edge cases - depth_min = self._get_spec_value("depth_min", None) - depth_max = self._get_spec_value("depth_max", None) - spatial_constraint = self._get_spec_value("spatial", True) + latlon_buffer = self.fetch_spec.latlon_buffer + depth_min = self.fetch_spec.depth_min + depth_max = self.fetch_spec.depth_max + spatial_constraint = self.fetch_spec.spatial min_lon_bound = self.min_lon - latlon_buffer if spatial_constraint else None max_lon_bound = self.max_lon + latlon_buffer if spatial_constraint else None @@ -186,7 +196,7 @@ def _generate_fieldset(self) -> parcels.FieldSet: fieldsets_list = [] keys = list(self.variables.keys()) - time_buffer = self._get_spec_value("time", 0.0) + time_buffer = self.fetch_spec.time_buffer for key in keys: var = self.variables[key] diff --git a/src/virtualship/instruments/ctd.py b/src/virtualship/instruments/ctd.py index 6f0eca3f..d6764130 100644 --- a/src/virtualship/instruments/ctd.py +++ b/src/virtualship/instruments/ctd.py @@ -7,7 +7,7 @@ from parcels import ParticleFile, ParticleSet, Variable from parcels._core.statuscodes import StatusCode -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.utils import ( @@ -142,9 +142,6 @@ class CTDInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize CTDInstrument.""" variables = expedition.instruments_config.ctd_config.active_variables() - fetch_spec = { - "spatial": True, - } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -152,7 +149,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) diff --git a/src/virtualship/instruments/drifter.py b/src/virtualship/instruments/drifter.py index f2d2351c..6319831e 100644 --- a/src/virtualship/instruments/drifter.py +++ b/src/virtualship/instruments/drifter.py @@ -8,7 +8,7 @@ from parcels._core.statuscodes import StatusCode from parcels.kernels import AdvectionRK2 -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.spacetime import Spacetime @@ -88,20 +88,17 @@ def __init__(self, expedition, from_data): "V": "vo", **sensor_variables, } # advection variables (U and V) are always required for drifter simulation; sensor variables come from config - fetch_spec = { - "spatial": True, - "latlon": 30.0, # [degrees]; TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) - "time": expedition.instruments_config.drifter_config.lifetime.total_seconds() - / ( - 24 * 3600 - ), # [days]; TODO: as above, can potentially be removed in the future - "depth_min": abs( + fetch_spec = FetchSpec( + latlon_buffer=30.0, # TODO: generous buffer to limit tmp file size download, can potentially be removed in the future as and when Parcels streaming performance improves (see #358) + time_buffer=expedition.instruments_config.drifter_config.lifetime.total_seconds() + / (24 * 3600), # [days] + depth_min=abs( expedition.instruments_config.drifter_config.depth_meter ), # [meters] - "depth_max": abs( + depth_max=abs( expedition.instruments_config.drifter_config.depth_meter ), # [meters] - } + ) super().__init__( expedition, diff --git a/src/virtualship/instruments/ship_underwater_st.py b/src/virtualship/instruments/ship_underwater_st.py index 1563668a..1dc7522a 100644 --- a/src/virtualship/instruments/ship_underwater_st.py +++ b/src/virtualship/instruments/ship_underwater_st.py @@ -5,7 +5,7 @@ import numpy as np from parcels import ParticleFile, ParticleSet -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.utils import ( @@ -67,11 +67,6 @@ def __init__(self, expedition, from_data): variables = ( expedition.instruments_config.ship_underwater_st_config.active_variables() ) - fetch_spec = { - "spatial": True, - "latlon": 0.25, # [degrees] - "time": 0.0, # [days] - } super().__init__( expedition, @@ -79,7 +74,7 @@ def __init__(self, expedition, from_data): add_bathymetry=False, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) diff --git a/src/virtualship/instruments/xbt.py b/src/virtualship/instruments/xbt.py index 9b0d6432..7046fde1 100644 --- a/src/virtualship/instruments/xbt.py +++ b/src/virtualship/instruments/xbt.py @@ -7,7 +7,7 @@ from parcels import ParticleFile, ParticleSet, Variable from parcels._core.statuscodes import StatusCode -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.sensors import SensorType from virtualship.instruments.types import InstrumentType from virtualship.models.spacetime import Spacetime @@ -95,9 +95,6 @@ class XBTInstrument(Instrument): def __init__(self, expedition, from_data): """Initialize XBTInstrument.""" variables = expedition.instruments_config.xbt_config.active_variables() - fetch_spec = { - "spatial": True, - } # lat/lon constrained to waypoint locations + buffer super().__init__( expedition, @@ -105,7 +102,7 @@ def __init__(self, expedition, from_data): add_bathymetry=True, allow_time_extrapolation=True, verbose_progress=False, - fetch_spec=fetch_spec, + fetch_spec=FetchSpec(), from_data=from_data, ) From cc0effafcd53a7320e0d06c62537fb2406416658 Mon Sep 17 00:00:00 2001 From: j-atkins <106238905+j-atkins@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:37:05 +0200 Subject: [PATCH 3/3] new test for FetchSpec class --- tests/instruments/test_base.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/instruments/test_base.py b/tests/instruments/test_base.py index a17f95bf..1b090cb9 100644 --- a/tests/instruments/test_base.py +++ b/tests/instruments/test_base.py @@ -2,11 +2,26 @@ import pytest -from virtualship.instruments.base import Instrument +from virtualship.instruments.base import FetchSpec, Instrument from virtualship.instruments.types import InstrumentType from virtualship.utils import get_instrument_class +def test_FetchSpec(): + fetch_spec = FetchSpec() + + # test that default values are set + assert fetch_spec.latlon_buffer is not None + assert fetch_spec.time_buffer is not None + + # test setting values (in new instance) and that original is unchanged in memory + fetch_spec2 = FetchSpec(latlon_buffer=0.5, time_buffer=1.0) + assert fetch_spec2.latlon_buffer == 0.5 + assert fetch_spec2.time_buffer == 1.0 + + assert fetch_spec.latlon_buffer != fetch_spec2.latlon_buffer + + def test_all_instruments_have_instrument_class(): for instrument in InstrumentType: instrument_class = get_instrument_class(instrument)