From 45c080c0b5879992ff8f24eb9f39d52a031208e6 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Mon, 18 May 2026 18:12:31 +0200 Subject: [PATCH 1/2] test: regression test for VerticalObservation/VerticalModelResult mutating input DataFrame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both classes call _parse_vertical_input, which currently sets data.index.name = "time" on a column-sliced view of the caller's DataFrame. Because pandas shares the Index object between a column slice and its source, this mutation leaks back to the caller — observed as df.index.name flipping from None to "time" after construction. These tests pin the contract that construction is non-mutating. They are expected to fail until the parser is fixed in a follow-up commit. --- tests/model/test_vertical.py | 11 +++++++++++ tests/observation/test_vertical_obs.py | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/model/test_vertical.py b/tests/model/test_vertical.py index 792e29d4b..f5a6a63bf 100644 --- a/tests/model/test_vertical.py +++ b/tests/model/test_vertical.py @@ -211,6 +211,17 @@ def test_vertical_model_roundtrip_from_dataset(self, vertical_model_df): mr.name = "changed_name" assert mr2.name == "salt_model" + def test_construction_does_not_mutate_input_dataframe(self, vertical_model_df): + assert vertical_model_df.index.name is None + original_columns = list(vertical_model_df.columns) + + ms.VerticalModelResult( + vertical_model_df, item="Salinity", z_item="z", x=12.0, y=55.0 + ) + + assert vertical_model_df.index.name is None + assert list(vertical_model_df.columns) == original_columns + # ================ # Test extract profile and align to obs profiles from dfsu # =============== diff --git a/tests/observation/test_vertical_obs.py b/tests/observation/test_vertical_obs.py index fb755c5ff..2d758f0df 100644 --- a/tests/observation/test_vertical_obs.py +++ b/tests/observation/test_vertical_obs.py @@ -250,6 +250,17 @@ def test_roundtrip_from_dataset_preserves_vertical_observation(self, _vertical_d assert obs2.attrs["station"] == "A" assert obs2.name == obs.name + def test_construction_does_not_mutate_input_dataframe(self, _vertical_df): + assert _vertical_df.index.name is None + original_columns = list(_vertical_df.columns) + + ms.VerticalObservation( + _vertical_df, item="value", z_item="z", x=12.0, y=55.0 + ) + + assert _vertical_df.index.name is None + assert list(_vertical_df.columns) == original_columns + def test_to_dataframe(self, _vertical_df): obs = ms.VerticalObservation( _vertical_df, From f4143fa251da17ef375bf9b28996b5805c52f772 Mon Sep 17 00:00:00 2001 From: Henrik Andersson Date: Mon, 18 May 2026 18:23:43 +0200 Subject: [PATCH 2/2] fix: do not mutate caller's DataFrame index in vertical parser Replace the in-place `data.index.name = "time"` with `rename_axis("time")`, which returns a new DataFrame. Pandas shares the Index object between a column slice and its source, so the previous code leaked the rename back to the caller. --- src/modelskill/timeseries/_vertical.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/modelskill/timeseries/_vertical.py b/src/modelskill/timeseries/_vertical.py index 57226ad58..29f16c85c 100644 --- a/src/modelskill/timeseries/_vertical.py +++ b/src/modelskill/timeseries/_vertical.py @@ -132,8 +132,7 @@ def _parse_vertical_input( if isinstance(data, mikeio.Dataset): ds = data.to_xarray() elif isinstance(data, pd.DataFrame): - data.index.name = "time" - ds = data.to_xarray() + ds = data.rename_axis("time").to_xarray() else: assert len(data.dims) == 1, "Only 0-dimensional data are supported" if data.coords[list(data.coords)[0]].name != "time":