From ba4024e3785bc542f863645997bec1d21b10f591 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Mon, 18 May 2026 22:36:01 +0200 Subject: [PATCH 1/2] Remove keep_duplicates from vertical observation and model result Raise ValueError on duplicate (time, z) entries instead of silently deduplicating. Vertical profiles must have a unique depth per timestamp; duplicates indicate a data-quality issue that the caller should resolve before construction, not a sampling artifact the library should paper over. --- src/modelskill/model/vertical.py | 9 +--- src/modelskill/obs.py | 4 -- src/modelskill/timeseries/_vertical.py | 19 +++----- tests/model/test_vertical.py | 56 +++++----------------- tests/observation/test_vertical_obs.py | 66 +------------------------- 5 files changed, 21 insertions(+), 133 deletions(-) diff --git a/src/modelskill/model/vertical.py b/src/modelskill/model/vertical.py index b1e529ccb..5b571469a 100644 --- a/src/modelskill/model/vertical.py +++ b/src/modelskill/model/vertical.py @@ -1,5 +1,5 @@ from __future__ import annotations -from typing import Any, Literal, Sequence +from typing import Any, Sequence import xarray as xr import pandas as pd @@ -35,11 +35,6 @@ class VerticalModelResult(TimeSeries): zonal coordinate of point position, inferred from data if not given, else None quantity : Quantity, optional Model quantity, for MIKE files this is inferred from the EUM information - keep_duplicates : (str, bool), optional - Strategy for handling duplicate timestamps (wraps xarray.Dataset.drop_duplicates) - "first" to keep first occurrence, "last" to keep last occurrence, - False to drop all duplicates, "offset" to add milliseconds to - consecutive duplicates, by default "first" aux_items : list[int | str] | None, optional Auxiliary items, by default None """ @@ -54,7 +49,6 @@ def __init__( z_item: str | int = 0, x: float | None = None, y: float | None = None, - keep_duplicates: Literal["first", "last", False] = "first", aux_items: Sequence[int | str] | None = None, ) -> None: if not self._is_input_validated(data): @@ -66,7 +60,6 @@ def __init__( z_item=z_item, x=x, y=y, - keep_duplicates=keep_duplicates, aux_items=aux_items, ) assert isinstance(data, xr.Dataset) diff --git a/src/modelskill/obs.py b/src/modelskill/obs.py index e4f653bbe..e06b79858 100644 --- a/src/modelskill/obs.py +++ b/src/modelskill/obs.py @@ -392,8 +392,6 @@ class VerticalObservation(Observation): User-defined name for identification in plots and summaries. weight : float, optional Weighting factor for skill scores, by default 1.0. - keep_duplicates : {"first", "last", False}, optional - Strategy for handling duplicate timestamps/z pairs. quantity : Quantity, optional Physical quantity metadata used for validation against model results. aux_items : list[int | str], optional @@ -445,7 +443,6 @@ def __init__( z_item: int | str | None = 0, name: str | None = None, weight: float = 1.0, - keep_duplicates: Literal["first", "last", False] = "first", quantity: Quantity | None = None, aux_items: list[int | str] | None = None, attrs: dict | None = None, @@ -460,7 +457,6 @@ def __init__( z_item=z_item, x=x, y=y, - keep_duplicates=keep_duplicates, ) assert isinstance(data, xr.Dataset) super().__init__(data=data, weight=weight, attrs=attrs) diff --git a/src/modelskill/timeseries/_vertical.py b/src/modelskill/timeseries/_vertical.py index 57226ad58..be3cd2f55 100644 --- a/src/modelskill/timeseries/_vertical.py +++ b/src/modelskill/timeseries/_vertical.py @@ -2,8 +2,7 @@ from collections.abc import Hashable from dataclasses import dataclass from pathlib import Path -from typing import Literal, get_args, Optional, List, Sequence -import warnings +from typing import get_args, Optional, List, Sequence import pandas as pd import xarray as xr from ._coords import XYZCoords @@ -88,7 +87,6 @@ def _parse_vertical_input( z_item: str | int | None, x: float | None = None, y: float | None = None, - keep_duplicates: Literal["first", "last", False] = "first", aux_items: Optional[Sequence[int | str]] = None, ) -> xr.Dataset: assert isinstance( @@ -142,16 +140,13 @@ def _parse_vertical_input( ds = ds.rename({sel_items.z: "z"}) - # keep first, last or none of duplicate (time, z) pairs idx_df = pd.DataFrame({"time": ds["time"].to_index(), "z": ds["z"].values}) - - keep_mask = ~idx_df.duplicated(subset=["time", "z"], keep=keep_duplicates) - - n_removed = int((~keep_mask).sum()) - ds = ds.isel(time=keep_mask.values) - if n_removed > 0: - warnings.warn( - f"Removed {n_removed} duplicate (time, z) entries with keep={keep_duplicates}" + n_duplicates = int(idx_df.duplicated(subset=["time", "z"]).sum()) + if n_duplicates > 0: + raise ValueError( + f"Input contains {n_duplicates} duplicate (time, z) entries. " + "Vertical profiles must have a unique depth per timestamp; " + "deduplicate the input before constructing the object." ) ds = ds.dropna(dim="time", subset=["z"]) # remove times with z as nan diff --git a/tests/model/test_vertical.py b/tests/model/test_vertical.py index 792e29d4b..7c86fe504 100644 --- a/tests/model/test_vertical.py +++ b/tests/model/test_vertical.py @@ -28,25 +28,6 @@ def vertical_model_df() -> pd.DataFrame: ) -@pytest.fixture -def vertical_model_df_duplicates() -> pd.DataFrame: - return pd.DataFrame( - { - "z": [-5.0, -5.0, -4.0, -4.0, -3.0], - "Salinity": [30.0, 300.0, 31.0, 310.0, 32.0], - }, - index=pd.to_datetime( - [ - "2019-01-01 00:00:00", - "2019-01-01 00:00:00", - "2019-01-01 00:00:00", - "2019-01-01 00:00:00", - "2019-01-01 01:00:00", - ] - ), - ) - - @pytest.fixture def vertical_model_df_aux() -> pd.DataFrame: return pd.DataFrame( @@ -136,38 +117,23 @@ def test_item_named_z(self, dfs0_ds): with pytest.raises(ValueError, match="name 'z' is reserved "): _ = ms.VerticalModelResult(ds_test) - # =============== - # test arguments options for handling duplicates - # =============== - @pytest.mark.parametrize( - "keep_duplicates,expected_removed,expected_z,expected_values", - [ - ("first", 2, [-5.0, -4.0, -3.0], [30.0, 31.0, 32.0]), - ("last", 2, [-5.0, -4.0, -3.0], [300.0, 310.0, 32.0]), - (False, 4, [-3.0], [32.0]), - ], - ) - def test_vertical_model_keep_duplicates_modes( - self, - vertical_model_df_duplicates, - keep_duplicates, - expected_removed, - expected_z, - expected_values, - ): - with pytest.warns(UserWarning, match=f"Removed {expected_removed} duplicate"): - mr = ms.VerticalModelResult( - vertical_model_df_duplicates, + def test_duplicate_time_z_pairs_raises(self): + df = pd.DataFrame( + { + "z": [-5.0, -4.0, -4.0], + "Salinity": [30.0, 31.0, 31.5], + }, + index=[pd.Timestamp("2019-01-01")] * 3, + ) + with pytest.raises(ValueError, match="duplicate \\(time, z\\) entries"): + ms.VerticalModelResult( + df, item="Salinity", z_item="z", x=12.0, y=55.0, - keep_duplicates=keep_duplicates, ) - assert list(mr.data["z"].values) == expected_z - assert list(mr.data[mr.name].values) == expected_values - # aux items def test_vertical_model_aux_items_preserved_and_tagged(self, vertical_model_df_aux): mr = ms.VerticalModelResult( diff --git a/tests/observation/test_vertical_obs.py b/tests/observation/test_vertical_obs.py index fb755c5ff..c84d26c0b 100644 --- a/tests/observation/test_vertical_obs.py +++ b/tests/observation/test_vertical_obs.py @@ -15,25 +15,6 @@ def _vertical_df() -> pd.DataFrame: ) -@pytest.fixture -def _vertical_df_duplicates() -> pd.DataFrame: - return pd.DataFrame( - { - "z": [-5.0, -5.0, -4.0, -4.0, -3.0], - "value": [1.0, 10.0, 2.0, 20.0, 7.0], - }, - index=pd.to_datetime( - [ - "2019-01-01 00:00:00", - "2019-01-01 00:00:00", - "2019-01-01 00:00:00", - "2019-01-01 00:00:00", - "2019-01-01 01:00:00", - ] - ), - ) - - @pytest.fixture def _vertical_df_aux() -> pd.DataFrame: return pd.DataFrame( @@ -114,7 +95,7 @@ def test_with_and_without_item_arg(self): fn = Path("tests/testdata/vertical/VerticalProfile_obs1.dfs0") assert isinstance(ms.observation(fn, z_item="z"), ms.VerticalObservation) - def test_duplicated_time_z_pairs(self): + def test_duplicate_time_z_pairs_raises(self): df = pd.DataFrame( { "z": [-5.0, -4.0, -4.0], @@ -122,7 +103,7 @@ def test_duplicated_time_z_pairs(self): }, index=[pd.Timestamp("2019-01-01")] * 3, ) - with pytest.warns(UserWarning, match="Removed 1 duplicate"): + with pytest.raises(ValueError, match="duplicate \\(time, z\\) entries"): ms.VerticalObservation( df, item="value", @@ -131,49 +112,6 @@ def test_duplicated_time_z_pairs(self): y=55.0, ) - def test_keep_duplicates_last_is_applied(self, _vertical_df_duplicates): - with pytest.warns(UserWarning, match="Removed 2 duplicate"): - obs = ms.VerticalObservation( - _vertical_df_duplicates, - item="value", - z_item="z", - x=12.0, - y=55.0, - keep_duplicates="last", - ) - - assert list(obs.data["z"].values) == [-5.0, -4.0, -3.0] - assert list(obs.data["value"].values) == [10.0, 20.0, 7.0] - - @pytest.mark.parametrize( - "keep_duplicates,expected_removed,expected_z,expected_values", - [ - ("first", 2, [-5.0, -4.0, -3.0], [1.0, 2.0, 7.0]), - ("last", 2, [-5.0, -4.0, -3.0], [10.0, 20.0, 7.0]), - (False, 4, [-3.0], [7.0]), - ], - ) - def test_keep_duplicates_modes( - self, - _vertical_df_duplicates, - keep_duplicates, - expected_removed, - expected_z, - expected_values, - ): - with pytest.warns(UserWarning, match=f"Removed {expected_removed} duplicate"): - obs = ms.VerticalObservation( - _vertical_df_duplicates, - item="value", - z_item="z", - x=12.0, - y=55.0, - keep_duplicates=keep_duplicates, - ) - - assert list(obs.data["z"].values) == expected_z - assert list(obs.data["value"].values) == expected_values - def test_single_item_input_raises(self): df = pd.DataFrame( {"value": [1.0, 1.1, 1.2]}, From 933a6e62b6df4513b37009cb2089b6c13b008948 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Mon, 18 May 2026 23:26:42 +0200 Subject: [PATCH 2/2] Require explicit item and z_item on VerticalObservation / VerticalModelResult Drops the positional defaults (`z_item=0`, `item=None` with 2-column auto-pick). A vertical profile's depth axis and value column are the defining inputs; defaulting them to "column 0" / "column 1" is a position-based assumption in an API that should be name-based. The same flaw exists on TrackObservation and TrackModelResult but those have real users and will get a separate deprecation PR; vertical is alpha and can break cleanly. Internal transforms (`sel`, `trim`, etc.) reach the constructors via the base `_create_new_instance` which calls `self.__class__(data)` with no kwargs. Two overrides reconstruct `item` from `self.name` and use `z_item="z"` (always renamed by the parser), preserving the prevalidated- dataset path without leaking sentinel defaults into the public signature. Two other internal call sites that bypassed `_create_new_instance` (`comparison/_comparison.py` and `comparison/_vertical_comparison.py`) now go through it for consistency. Also fixes a docstring typo (`mikeio.Dfs0, mikeio.Dfs0` -> `mikeio.Dfs0, mikeio.Dataset`) and switches both data-arg docstrings from type-listing to semantic description. --- src/modelskill/comparison/_comparison.py | 2 +- .../comparison/_vertical_comparison.py | 2 +- src/modelskill/model/vertical.py | 24 ++++++----- src/modelskill/obs.py | 22 +++++----- src/modelskill/timeseries/_vertical.py | 17 +++----- tests/model/test_vertical.py | 21 +++++----- tests/observation/test_vertical_obs.py | 40 +++++++++---------- tests/test_match.py | 28 ++++++++----- 8 files changed, 84 insertions(+), 72 deletions(-) diff --git a/src/modelskill/comparison/_comparison.py b/src/modelskill/comparison/_comparison.py index aed4cc27e..7bb1c2b16 100644 --- a/src/modelskill/comparison/_comparison.py +++ b/src/modelskill/comparison/_comparison.py @@ -948,7 +948,7 @@ def sel( m_data = v.data.where( (v.data["z"] >= lo) & (v.data["z"] <= hi), drop=True ) - raw_mod[k] = type(v)(m_data) # type: ignore[call-arg] + raw_mod[k] = v._create_new_instance(m_data) raw_mod_data = raw_mod else: z_mask = xr.apply_ufunc(np.isclose, d["z"], float(z)) diff --git a/src/modelskill/comparison/_vertical_comparison.py b/src/modelskill/comparison/_vertical_comparison.py index c6f2a6918..443bcecdf 100644 --- a/src/modelskill/comparison/_vertical_comparison.py +++ b/src/modelskill/comparison/_vertical_comparison.py @@ -366,4 +366,4 @@ def _raw_model_to_z(self, raw_mod, z): z_dist.reset_index().groupby("time", sort=False)["z"].idxmin().to_numpy() ) sel_data = raw_mod.data.isel(time=np.sort(nearest_idx)) - return type(raw_mod)(sel_data) + return raw_mod._create_new_instance(sel_data) diff --git a/src/modelskill/model/vertical.py b/src/modelskill/model/vertical.py index 5b571469a..915fd6ba4 100644 --- a/src/modelskill/model/vertical.py +++ b/src/modelskill/model/vertical.py @@ -1,5 +1,6 @@ from __future__ import annotations from typing import Any, Sequence +from typing_extensions import Self import xarray as xr import pandas as pd @@ -19,16 +20,17 @@ class VerticalModelResult(TimeSeries): Parameters ---------- - data : str, Path, pd.DataFrame, mikeio.Dfs0, mikeio.Dfs0, xr.Dataset - The input data or file path + data : dfs0 path or in-memory profile data + Path to a dfs0 file, or a long-format DataFrame / mikeio.Dataset / + xr.Dataset with a time index, a vertical-coordinate column, and one + or more value columns. + item : str | int + Index or name of the primary model item. + z_item : str | int + Index or name of the vertical coordinate item. name : str | None, optional The name of the model result, by default None (will be set to file name or item name) - item : str | int | None, optional - If multiple items/arrays are present in the input an item - must be given (as either an index or a string), by default None - z_item : str | int | None, optional - Item of the first coordinate of positions, by default None x : float, optional lateral coordinate of point position, inferred from data if not given, else None y : float, optional @@ -43,10 +45,10 @@ def __init__( self, data: VerticalType, *, + item: str | int, + z_item: str | int, name: str | None = None, - item: str | int | None = None, quantity: Quantity | None = None, - z_item: str | int = 0, x: float | None = None, y: float | None = None, aux_items: Sequence[int | str] | None = None, @@ -72,6 +74,10 @@ def z(self) -> Any: """z-coordinate""" return self._coordinate_values("z") + def _create_new_instance(self, data: xr.Dataset) -> Self: + """Reconstruct instance from a modelskill-built dataset.""" + return self.__class__(data, item=self.name, z_item="z") + def _match_to_nearest_times( self, obs_df: pd.DataFrame, t_tol: pd.Timedelta | None = None ) -> pd.DataFrame: diff --git a/src/modelskill/obs.py b/src/modelskill/obs.py index e06b79858..96b5284ca 100644 --- a/src/modelskill/obs.py +++ b/src/modelskill/obs.py @@ -374,20 +374,20 @@ class VerticalObservation(Observation): Parameters ---------- - data : (str, Path, pd.DataFrame, mikeio.Dfs0, mikeio.Dataset, xr.Dataset) - Input data with vertical profile observations. - item : int or str, optional + data : dfs0 path or in-memory profile data + Path to a dfs0 file, or a long-format DataFrame / mikeio.Dataset / + xr.Dataset with a time index, a vertical-coordinate column, and one + or more value columns. + item : int or str Index or name of the primary observation item. - If the input contains more than one candidate value item, - this argument must be provided. + z_item : int or str + Index or name of the vertical coordinate item. x : float, optional x-coordinate of the observation location. If not provided, it is inferred from data when possible. y : float, optional y-coordinate of the observation location. If not provided, it is inferred from data when possible. - z_item : int or str, optional - Index or name of the vertical coordinate item, by default 0. name : str, optional User-defined name for identification in plots and summaries. weight : float, optional @@ -437,10 +437,10 @@ def __init__( self, data: VerticalType, *, - item: int | str | None = None, + item: int | str, + z_item: int | str, x: float | None = None, y: float | None = None, - z_item: int | str | None = 0, name: str | None = None, weight: float = 1.0, quantity: Quantity | None = None, @@ -465,6 +465,10 @@ def __init__( def z(self): return self._coordinate_values("z") + def _create_new_instance(self, data: xr.Dataset) -> Self: + """Reconstruct instance from a modelskill-built dataset.""" + return self.__class__(data, item=self.name, z_item="z") + class NodeObservation(Observation): """Class for observations at network nodes. diff --git a/src/modelskill/timeseries/_vertical.py b/src/modelskill/timeseries/_vertical.py index be3cd2f55..fe0a5990b 100644 --- a/src/modelskill/timeseries/_vertical.py +++ b/src/modelskill/timeseries/_vertical.py @@ -29,22 +29,15 @@ def all(self) -> List[str]: def _parse_vertical_items( items: Sequence[Hashable], - z_item: int | str | None, - item: int | str | None, + z_item: int | str, + item: int | str, aux_items: Optional[Sequence[int | str]] = None, ) -> VerticalItem: - """If input has exactly 2 items we accept item=None""" + """Resolve and validate item selection from available column names.""" if len(items) < 2: raise ValueError( f"Input has only {len(items)} items. It should have at least 2." ) - if item is None: - if len(items) == 2: - item = 1 - elif len(items) > 2: - raise ValueError( - f"Input has more than 2 items, but item was not given! Available items: {items}" - ) item = _get_name(item, valid_names=items) z_item = _get_name(z_item, valid_names=items) @@ -82,9 +75,9 @@ def _include_location( def _parse_vertical_input( data: VerticalType, name: Optional[str], - item: str | int | None, + item: str | int, quantity: Optional[Quantity], - z_item: str | int | None, + z_item: str | int, x: float | None = None, y: float | None = None, aux_items: Optional[Sequence[int | str]] = None, diff --git a/tests/model/test_vertical.py b/tests/model/test_vertical.py index 7c86fe504..4d467f235 100644 --- a/tests/model/test_vertical.py +++ b/tests/model/test_vertical.py @@ -102,20 +102,21 @@ def test_open_with_factory(self, dfs0_fpath): # ================ # Test failing and optional args # ================ - # failing without z_item - def test_fail_with_3_items_no_item_arg(self, dfs0_ds): - ds_test = dfs0_ds.copy() - ds_test["extra_item"] = ds_test[1].copy() - with pytest.raises(ValueError, match="Input has more than 2 items, but"): - _ = ms.VerticalModelResult(ds_test) - - # failing z wronge location + def test_missing_item_kwarg_raises(self, dfs0_ds): + with pytest.raises(TypeError, match="item"): + ms.VerticalModelResult(dfs0_ds, z_item="z") + + def test_missing_z_item_kwarg_raises(self, dfs0_ds): + with pytest.raises(TypeError, match="z_item"): + ms.VerticalModelResult(dfs0_ds, item="Salinity") + + # failing z wrong location def test_item_named_z(self, dfs0_ds): ds_test = mikeio.Dataset( [dfs0_ds[1], dfs0_ds[0]], ) with pytest.raises(ValueError, match="name 'z' is reserved "): - _ = ms.VerticalModelResult(ds_test) + _ = ms.VerticalModelResult(ds_test, item="z", z_item="Salinity") def test_duplicate_time_z_pairs_raises(self): df = pd.DataFrame( @@ -167,7 +168,7 @@ def test_vertical_model_roundtrip_from_dataset(self, vertical_model_df): y=55.0, name="salt_model", ) - mr2 = ms.VerticalModelResult(mr.data) + mr2 = ms.VerticalModelResult(mr.data, item="Salinity", z_item="z") assert mr.equals(mr2) assert mr2.gtype == mr.gtype diff --git a/tests/observation/test_vertical_obs.py b/tests/observation/test_vertical_obs.py index c84d26c0b..8507d47cc 100644 --- a/tests/observation/test_vertical_obs.py +++ b/tests/observation/test_vertical_obs.py @@ -73,7 +73,11 @@ def test_sel_by_z_scalar(self, _vertical_df): ) # out = obs.sel(z=-4.0) # only works with xarray >= 2026.01.0 - out = ms.VerticalObservation(obs.data.where(obs.data["z"] == -4.0, drop=True)) + out = ms.VerticalObservation( + obs.data.where(obs.data["z"] == -4.0, drop=True), + item="value", + z_item="z", + ) assert isinstance(out, ms.VerticalObservation) assert len(out.data) == 1 @@ -81,20 +85,11 @@ def test_sel_by_z_scalar(self, _vertical_df): def test_open_dfs0_equal(self): fn = Path("tests/testdata/vertical/VerticalProfile_obs2.dfs0") - obs = ms.observation(fn, z_item="z") - obs2 = ms.VerticalObservation(fn) + obs = ms.observation(fn, item="Salinity", z_item="z") + obs2 = ms.VerticalObservation(fn, item="Salinity", z_item="z") assert isinstance(obs, ms.VerticalObservation) assert obs.equals(obs2) - def test_with_and_without_item_arg(self): - fn = Path("tests/testdata/vertical/VerticalProfile_ST.dfs0") - # no item specified, but multiple items in file - with pytest.raises(ValueError): - ms.observation(fn, z_item="z") - # below should be fine...only one item - fn = Path("tests/testdata/vertical/VerticalProfile_obs1.dfs0") - assert isinstance(ms.observation(fn, z_item="z"), ms.VerticalObservation) - def test_duplicate_time_z_pairs_raises(self): df = pd.DataFrame( { @@ -121,19 +116,22 @@ def test_single_item_input_raises(self): with pytest.raises(ValueError, match="at least 2"): ms.VerticalObservation(df, item="value", z_item="z", x=12.0, y=55.0) - def test_more_than_two_items_without_item_raises(self): + def test_missing_item_kwarg_raises(self): df = pd.DataFrame( - { - "z": [-5.0, -4.0, -3.0], - "value1": [1.0, 1.1, 1.2], - "value2": [2.0, 2.1, 2.2], - }, + {"z": [-5.0, -4.0, -3.0], "value": [1.0, 1.1, 1.2]}, index=[pd.Timestamp("2019-01-01")] * 3, ) - - with pytest.raises(ValueError, match="item was not given"): + with pytest.raises(TypeError, match="item"): ms.VerticalObservation(df, z_item="z", x=12.0, y=55.0) + def test_missing_z_item_kwarg_raises(self): + df = pd.DataFrame( + {"z": [-5.0, -4.0, -3.0], "value": [1.0, 1.1, 1.2]}, + index=[pd.Timestamp("2019-01-01")] * 3, + ) + with pytest.raises(TypeError, match="z_item"): + ms.VerticalObservation(df, item="value", x=12.0, y=55.0) + def test_duplicate_item_specification_raises(self, _vertical_df_aux): with pytest.raises(ValueError, match="Duplicate items"): ms.VerticalObservation( @@ -181,7 +179,7 @@ def test_roundtrip_from_dataset_preserves_vertical_observation(self, _vertical_d y=55.0, attrs={"station": "A"}, ) - obs2 = ms.VerticalObservation(obs.data) + obs2 = ms.VerticalObservation(obs.data, item="value", z_item="z") assert obs.equals(obs2) assert obs2.attrs["gtype"] == obs.attrs["gtype"] diff --git a/tests/test_match.py b/tests/test_match.py index 5664c341b..a55cfd23d 100644 --- a/tests/test_match.py +++ b/tests/test_match.py @@ -7,6 +7,7 @@ import modelskill as ms from modelskill.comparison._comparison import ItemSelection from modelskill.model.dfsu import DfsuModelResult + try: from modelskill.network import _make_basic_network except ImportError: @@ -82,12 +83,16 @@ class TestVerticalObservation: @pytest.fixture(scope="class") def o4(self): fn = "tests/testdata/vertical/VerticalProfile_obs1.dfs0" - return ms.VerticalObservation(fn, z_item="z", name="vobs", x=657500, y=6553600) + return ms.VerticalObservation( + fn, item="Salinity", z_item="z", name="vobs", x=657500, y=6553600 + ) @pytest.fixture(scope="class") def mr4(self): fn = "tests/testdata/vertical/VerticalModel_at_obs.dfs0" - return ms.model_result(fn, item="Salinity", name="vmod", gtype="vertical") + return ms.model_result( + fn, item="Salinity", z_item="z", name="vmod", gtype="vertical" + ) @pytest.fixture(scope="class") def mr5(self): @@ -262,7 +267,7 @@ def test_align_unevently_sampled_obs(self, simple_vo, simple_vm): # drop the last point and modify last obs depth to -4 and create new VerticalObservation df_o = simple_vo.to_dataframe().iloc[0:-1, :] df_o.iloc[-1, 0] = -4 - vo = ms.VerticalObservation(df_o) + vo = ms.VerticalObservation(df_o, item=simple_vo.name, z_item="z") cmp = ms.match(vo, simple_vm) expected_mod_values = [1.1, 2.1, 4.2] expected_depths = simple_vo.data["z"].to_numpy().copy()[0:-1] @@ -273,7 +278,9 @@ def test_align_unevently_sampled_obs(self, simple_vo, simple_vm): def test_align_vertical_intepolation(self, simple_vm): # Create observations from model df - vo = ms.VerticalObservation(simple_vm.to_dataframe()) + vo = ms.VerticalObservation( + simple_vm.to_dataframe(), item=simple_vm.name, z_item="z" + ) # Modify model to be inbetween the obs depths and create new VerticalModelResult df = simple_vm.to_dataframe() df["z"] = df["z"] - 0.5 @@ -281,7 +288,7 @@ def test_align_vertical_intepolation(self, simple_vm): # mod_z = [-1.5, -2.5, -3.5, -4.5, -1.5, -2.5, -3.5, -4.5], # mod_v = [1.1, 2.1, 3.1, 4.1, 1.2, 2.2, 3.2, 4.2] # obs_z = [-1, -2, -3, -4, -1, -2, -3, -4] - vm = ms.VerticalModelResult(df) + vm = ms.VerticalModelResult(df, item=simple_vm.name, z_item="z") cmp = ms.match(vo, vm) # first depth is nan in models becasuse obs outside model domain @@ -292,7 +299,9 @@ def test_align_vertical_intepolation(self, simple_vm): assert cmp.data["mod"].to_numpy() == pytest.approx(expected_mod_values) def test_same_results(self, simple_vm): - vo = ms.VerticalObservation(simple_vm.to_dataframe()) + vo = ms.VerticalObservation( + simple_vm.to_dataframe(), item=simple_vm.name, z_item="z" + ) cmp = ms.match(vo, simple_vm) assert cmp.n_points == 8 assert cmp.data["mod"].to_numpy() == pytest.approx( @@ -323,7 +332,7 @@ def test_no_overlap_in_z(self, simple_vo, simple_vm): # shift model to be outside obs range df = simple_vm.to_dataframe() df["z"] = df["z"] - 2 - vm = ms.VerticalModelResult(df) + vm = ms.VerticalModelResult(df, item=simple_vm.name, z_item="z") cmp = ms.match(simple_vo, vm) assert cmp.n_points == 0 @@ -331,7 +340,7 @@ def test_only_1_model_depth_overlap(self, simple_vo, simple_vm): # shift model to be outside obs range except for one point df = simple_vm.to_dataframe() df["z"] = df["z"] - 1 # only match with models at z=-2 exact - vm = ms.VerticalModelResult(df) + vm = ms.VerticalModelResult(df, item=simple_vm.name, z_item="z") cmp = ms.match(simple_vo, vm) assert cmp.n_points == 2 @@ -1044,7 +1053,8 @@ def test_network_match_multi_obs_multi_model_comprehensive( def test_network_match_error_non_node_observation(network_mr, point_obs_error): """Test that non-NodeObservation raises appropriate error""" with pytest.raises( - TypeError, match="NetworkModelResult supports NodeObservation and ReachObservation" + TypeError, + match="NetworkModelResult supports NodeObservation and ReachObservation", ): ms.match(point_obs_error, network_mr)