From fe1f4ab78644d6e2018d6d73e93060fc3a88ebbf Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Tue, 19 Dec 2023 18:54:27 +0100 Subject: [PATCH 1/2] Avoid accidental mismatch --- modelskill/matching.py | 6 +++++ tests/test_match.py | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/modelskill/matching.py b/modelskill/matching.py index 31f8f71b5..e7a15159e 100644 --- a/modelskill/matching.py +++ b/modelskill/matching.py @@ -216,6 +216,12 @@ def match( assert isinstance(obs, Iterable) + if len(obs) > 1 and isinstance(mod, Iterable) and len(mod) > 1: + if not all(isinstance(m, (DfsuModelResult, GridModelResult)) for m in mod): + raise ValueError( + "Multiple models can only be compared if they are the the observation can be extracted, e.g. DfsuModelResult or GridModelResult" + ) + clist = [ _single_obs_compare( o, diff --git a/tests/test_match.py b/tests/test_match.py index 38f3fc890..bc564a986 100644 --- a/tests/test_match.py +++ b/tests/test_match.py @@ -457,3 +457,55 @@ def test_mod_aux_items_must_be_unique(): assert "wind_speed" in str(e.value) assert "remote" in str(e.value) + + +def test_multiple_obs_not_allowed_with_non_spatial_modelresults(): + o1 = ms.PointObservation( + pd.DataFrame( + {"wl": [1.0, 2.0]}, index=pd.date_range("2000", freq="H", periods=2) + ), + name="o1", + x=1, + y=2, + ) + o2 = ms.PointObservation( + pd.DataFrame( + {"wl": [1.0, 2.0]}, index=pd.date_range("2000", freq="H", periods=2) + ), + name="o2", + x=2, + y=3, + ) + m1 = ms.PointModelResult( + pd.DataFrame( + {"wl": [1.0, 2.0]}, index=pd.date_range("2000", freq="H", periods=2) + ), + name="m1", + x=1, + y=2, + ) + m2 = ms.PointModelResult( + pd.DataFrame( + {"wl": [1.0, 2.0]}, index=pd.date_range("2000", freq="H", periods=2) + ), + name="m2", + x=2, + y=3, + ) + m3 = ms.PointModelResult( + pd.DataFrame( + {"wl": [1.0, 2.0]}, index=pd.date_range("2000", freq="H", periods=2) + ), + name="m3", + x=3, + y=4, + ) + + # a single observation and model is ok + cmp = ms.match(obs=o1, mod=[m1, m2]) + assert "m1" in cmp.mod_names + assert "m2" in cmp.mod_names + + # but this is not allowed + with pytest.raises(ValueError, match="extracted"): + ms.match(obs=[o1, o2], mod=[m1, m2, m3]) From 55793a7e4979273ef305e74d8d3f70693257c192 Mon Sep 17 00:00:00 2001 From: Jesper Sandvig Mariegaard <34088801+jsmariegaard@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:21:52 +0000 Subject: [PATCH 2/2] expand error message --- modelskill/matching.py | 10 +++++++++- tests/test_match.py | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/modelskill/matching.py b/modelskill/matching.py index e7a15159e..fa6c32127 100644 --- a/modelskill/matching.py +++ b/modelskill/matching.py @@ -219,7 +219,15 @@ def match( if len(obs) > 1 and isinstance(mod, Iterable) and len(mod) > 1: if not all(isinstance(m, (DfsuModelResult, GridModelResult)) for m in mod): raise ValueError( - "Multiple models can only be compared if they are the the observation can be extracted, e.g. DfsuModelResult or GridModelResult" + """ + In case of multiple observations, multiple models can _only_ + be matched if they are _all_ of SpatialField type, e.g. DfsuModelResult + or GridModelResult. + + If you want match multiple point observations with multiple point model results, + please match one observation at a time and then create a collection of these + using modelskill.ComparerCollection(cmp_list) afterwards. The same applies to track data. + """ ) clist = [ diff --git a/tests/test_match.py b/tests/test_match.py index bc564a986..359a7be2d 100644 --- a/tests/test_match.py +++ b/tests/test_match.py @@ -507,5 +507,5 @@ def test_multiple_obs_not_allowed_with_non_spatial_modelresults(): assert "m2" in cmp.mod_names # but this is not allowed - with pytest.raises(ValueError, match="extracted"): + with pytest.raises(ValueError, match="SpatialField type"): ms.match(obs=[o1, o2], mod=[m1, m2, m3])