From 1aace354c7b92cd80a0f0da3e61da2e09d33caab Mon Sep 17 00:00:00 2001 From: Daniel Weindl Date: Wed, 14 Oct 2026 19:53:08 +0200 Subject: [PATCH] Fix spurious `index` column in v2 table round-trips `from_df` of the v2 parameter, measurement, observable and mapping tables unconditionally called `reset_index()`, which turned an unnamed index into an extra `index` column that was kept and written out. Now only named index levels are moved to columns. Also: * accept empty `observableParameters` / `noiseParameters` strings, so `MeasurementTable.from_df(table.to_df())` works * don't modify the input DataFrame in `ObservableTable.from_df` Fixes #541. Relates to #523. Co-Authored-By: Claude Opus 5.5 --- petab/v2/core.py | 21 +++++++--- tests/v2/test_core.py | 97 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 6 deletions(-) diff --git a/petab/v2/core.py b/petab/v2/core.py index 985b9b0d..5970a2e3 100644 --- a/petab/v2/core.py +++ b/petab/v2/core.py @@ -116,6 +116,14 @@ def _convert_nan_to_none(v): return v +def _reset_index(df: pd.DataFrame) -> pd.DataFrame: + """Move named index levels to columns and drop unnamed ones. + + Avoids spurious ``index`` columns for DataFrames with a default index. + """ + return df.reset_index(drop=all(name is None for name in df.index.names)) + + def _valid_petab_id(v: str) -> str: """Field validator for PEtab IDs.""" if not v: @@ -414,10 +422,10 @@ def from_df(cls, df: pd.DataFrame, **kwargs) -> ObservableTable: if df is None: return cls(**kwargs) - df = get_observable_df(df) + df = get_observable_df(df.copy()) observables = [ Observable(**row.to_dict()) - for _, row in df.reset_index().iterrows() + for _, row in _reset_index(df).iterrows() ] return cls(observables, **kwargs) @@ -789,7 +797,7 @@ def convert_nan_to_none(cls, v, info: ValidationInfo): ) @classmethod def _sympify_list(cls, v): - if v is None: + if v is None or v == "": return [] if isinstance(v, float) and np.isnan(v): @@ -817,6 +825,7 @@ def from_df(cls, df: pd.DataFrame, **kwargs) -> MeasurementTable: if df is None: return cls(**kwargs) + df = _reset_index(df) if C.MODEL_ID in df.columns: df[C.MODEL_ID] = df[C.MODEL_ID].apply(_convert_nan_to_none) @@ -824,7 +833,7 @@ def from_df(cls, df: pd.DataFrame, **kwargs) -> MeasurementTable: Measurement( **row.to_dict(), ) - for _, row in df.reset_index().iterrows() + for _, row in df.iterrows() ] return cls(measurements, **kwargs) @@ -896,7 +905,7 @@ def from_df(cls, df: pd.DataFrame, **kwargs) -> MappingTable: return cls(**kwargs) mappings = [ - Mapping(**row.to_dict()) for _, row in df.reset_index().iterrows() + Mapping(**row.to_dict()) for _, row in _reset_index(df).iterrows() ] return cls(mappings, **kwargs) @@ -1106,7 +1115,7 @@ def from_df(cls, df: pd.DataFrame, **kwargs) -> ParameterTable: parameters = [ Parameter(**row.to_dict()) - for _, row in df.reset_index().iterrows() + for _, row in _reset_index(df).iterrows() ] return cls(parameters, **kwargs) diff --git a/tests/v2/test_core.py b/tests/v2/test_core.py index 06cabe60..398a0e6f 100644 --- a/tests/v2/test_core.py +++ b/tests/v2/test_core.py @@ -64,6 +64,103 @@ def test_condition_table_round_trip(): assert conditions == conditions2 +@pytest.mark.parametrize( + "table_cls,df", + [ + ( + ParameterTable, + pd.DataFrame( + { + PARAMETER_ID: ["k1", "k2"], + LOWER_BOUND: [0.1, 0.1], + UPPER_BOUND: [10.0, 10.0], + NOMINAL_VALUE: [1.0, 2.0], + ESTIMATE: [True, False], + } + ), + ), + ( + ObservableTable, + pd.DataFrame( + { + OBSERVABLE_ID: ["obs1", "obs2"], + OBSERVABLE_FORMULA: ["x", "y"], + NOISE_FORMULA: ["1", "2"], + } + ), + ), + ( + MeasurementTable, + pd.DataFrame( + { + C.OBSERVABLE_ID: ["obs1", "obs2"], + C.TIME: [0.0, 1.0], + C.MEASUREMENT: [1.0, 2.0], + } + ), + ), + ( + MappingTable, + pd.DataFrame( + { + PETAB_ENTITY_ID: ["a", "b"], + MODEL_ENTITY_ID: ["1a", "1b"], + } + ), + ), + ], +) +def test_from_df_ignores_unnamed_index(table_cls, df): + """Unnamed indices must not end up as extra columns.""" + # non-default unnamed index + df.index = [5, 7] + df_orig = df.copy() + table = table_cls.from_df(df) + assert_frame_equal(df, df_orig) + out_df = table.to_df() + cols = set(out_df.columns) | set(filter(None, out_df.index.names)) + assert not cols & {"index", "level_0"} + assert table_cls.from_df(out_df) == table + + # a named index is kept as a column + id_col = df.columns[0] + table2 = table_cls.from_df(df.set_index(id_col)) + assert table2 == table + + +@pytest.mark.filterwarnings( + "ignore:.*Parameter scales are not supported in PEtab v2.*:UserWarning" +) +def test_problem_to_files_round_trip_columns(): + """Saving and reloading a problem must not add columns.""" + with tempfile.TemporaryDirectory() as tmp_dir: + petab1to2(example_dir_fujita / "Fujita.yaml", tmp_dir) + yaml_file = Path(tmp_dir, "Fujita.yaml") + problem = Problem.from_yaml(yaml_file) + + def get_columns(p: Problem): + return { + name: set(getattr(p, name).columns) + | set(filter(None, getattr(p, name).index.names)) + for name in ( + "parameter_df", + "measurement_df", + "observable_df", + "condition_df", + "experiment_df", + ) + } + + expected = get_columns(problem) + for i in range(2): + out_dir = Path(tmp_dir, f"round{i}") + problem.to_files(out_dir) + problem = Problem.from_yaml(out_dir / yaml_file.name) + assert get_columns(problem) == expected + for cols in expected.values(): + assert not cols & {"index", "level_0"} + + @pytest.mark.filterwarnings( "ignore:.*Parameter scales are not supported in PEtab v2.*:UserWarning" )