diff --git a/CHANGELOG.md b/CHANGELOG.md index cf3da74d..79db7e5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,10 @@ unused ID, and accept switching times that conditions set from parameters fixed by the parameter table (by @FFroehlich in https://github.com/PEtab-dev/libpetab-python/pull/574) +* `petab1to2` now renames a pre-existing `experimentId` column in the + measurement table to `experimentId_v1*` instead of a random name, and + warns about it + (by @dweindl in https://github.com/PEtab-dev/libpetab-python/pull/572) **Features** * `petab1to2` and `petab_files_1to2` can skip validation via `validate=False` diff --git a/petab/v2/petab1to2.py b/petab/v2/petab1to2.py index bd7f9e44..b03cf714 100644 --- a/petab/v2/petab1to2.py +++ b/petab/v2/petab1to2.py @@ -9,7 +9,6 @@ from pathlib import Path from tempfile import TemporaryDirectory from urllib.parse import urlparse -from uuid import uuid4 import pandas as pd from pandas.io.common import get_handle, is_url @@ -227,12 +226,35 @@ def create_experiment_id(sim_cond_id: str, preeq_cond_id: str) -> str: v2.get_experiment_df(pd.DataFrame(experiments)), exp_table_path ) - for measurement_file in new_yaml_config.measurement_files: - measurement_df = v1.get_measurement_df(get_src_path(measurement_file)) - # if there is already an experiment ID column, we rename it + measurement_dfs = { + measurement_file: v1.get_measurement_df(get_src_path(measurement_file)) + for measurement_file in new_yaml_config.measurement_files + } + # if there is already an experiment ID column, we rename it, + # consistently across all measurement tables + if any( + v2.C.EXPERIMENT_ID in df.columns for df in measurement_dfs.values() + ): + all_columns = set().union( + *(df.columns for df in measurement_dfs.values()) + ) + renamed_experiment_id = f"{v2.C.EXPERIMENT_ID}_v1" + i = 1 + while renamed_experiment_id in all_columns: + i += 1 + renamed_experiment_id = f"{v2.C.EXPERIMENT_ID}_v1_{i}" + warnings.warn( + f"The measurement table has a column `{v2.C.EXPERIMENT_ID}`, " + "which is reserved in PEtab v2. " + f"It was renamed to `{renamed_experiment_id}`.", + # point to the caller of `petab1to2` + stacklevel=3, + ) + + for measurement_file, measurement_df in measurement_dfs.items(): if v2.C.EXPERIMENT_ID in measurement_df.columns: measurement_df = measurement_df.rename( - columns={v2.C.EXPERIMENT_ID: f"experiment_id_{uuid4()}"}, + columns={v2.C.EXPERIMENT_ID: renamed_experiment_id} ) # add pre-eq condition id if not present or convert to string # for simplicity diff --git a/tests/v2/test_conversion.py b/tests/v2/test_conversion.py index f06ece64..1a2a528f 100644 --- a/tests/v2/test_conversion.py +++ b/tests/v2/test_conversion.py @@ -4,6 +4,7 @@ import numpy as np import pandas as pd import pytest +import yaml from petab import v1, v2 from petab.v1.calculate import calculate_llh as calculate_llh_v1 @@ -612,6 +613,66 @@ def test_petab1to2_validate(tmp_path): assert len(problem.measurements) == 2 +@pytest.mark.parametrize("clash", [False, True]) +def test_petab1to2_existing_experiment_id_column(tmp_path, clash): + """Test that a pre-existing `experimentId` column is renamed + deterministically and consistently across measurement files.""" + yaml_file = _write_v1_problem( + tmp_path / "v1", + observables=[ + { + v1.C.OBSERVABLE_ID: "obs_B", + v1.C.OBSERVABLE_FORMULA: "B", + v1.C.NOISE_FORMULA: 0.1, + } + ], + measurements=[ + { + v1.C.OBSERVABLE_ID: "obs_B", + v1.C.SIMULATION_CONDITION_ID: "c0", + v1.C.TIME: 1.0, + v1.C.MEASUREMENT: 0.5, + v2.C.EXPERIMENT_ID: "dose_response", + } + ], + parameters=[_K1], + ) + # second measurement file, which may have a clashing column + yaml_path = tmp_path / "v1" / yaml_file + yaml_config = yaml.safe_load(yaml_path.read_text()) + measurement_df = pd.DataFrame( + { + v1.C.OBSERVABLE_ID: ["obs_B"], + v1.C.SIMULATION_CONDITION_ID: ["c0"], + v1.C.TIME: [2.0], + v1.C.MEASUREMENT: [0.5], + v2.C.EXPERIMENT_ID: ["time_course"], + } + ) + if clash: + measurement_df[f"{v2.C.EXPERIMENT_ID}_v1"] = "taken" + v1.write_measurement_df(measurement_df, tmp_path / "v1" / "meas2.tsv") + yaml_config["problems"][0]["measurement_files"].append("meas2.tsv") + yaml_path.write_text(yaml.safe_dump(yaml_config)) + + expected_name = f"{v2.C.EXPERIMENT_ID}_v1" + ("_2" if clash else "") + with pytest.warns(UserWarning, match=f"renamed to `{expected_name}`"): + problem = petab1to2(yaml_path) + + measurement_df = problem.measurement_df + assert measurement_df[expected_name].tolist() == [ + "dose_response", + "time_course", + ] + assert ( + measurement_df[v2.C.EXPERIMENT_ID].tolist() == ["experiment____c0"] * 2 + ) + if clash: + taken = measurement_df[f"{v2.C.EXPERIMENT_ID}_v1"] + assert pd.isna(taken.iloc[0]) + assert taken.iloc[1] == "taken" + + try: import benchmark_models_petab @@ -634,6 +695,9 @@ def test_petab1to2_validate(tmp_path): @pytest.mark.filterwarnings( "ignore:.*does not support log10-transformed observables.*:UserWarning" ) +@pytest.mark.filterwarnings( + "ignore:.*which is reserved in PEtab v2.*:UserWarning" +) @parametrize_or_skip def test_benchmark_collection(problem_id): """Test that we can upgrade all benchmark collection models."""