petab1to2: deterministic name for pre-existing experimentId column

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`

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

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."""