Skip to content

Commit ce80233

Browse files
dweindlclaude
andauthored
petab1to2: deterministic name for pre-existing experimentId column (#572)
`petab1to2` renamed a pre-existing `experimentId` column in v1 measurement tables to `experiment_id_<uuid4>`, so repeated conversions of the same problem (e.g., `Brannmark_JBC2010`, `Isensee_JCB2018`) produced different output. Now the column is renamed to `experimentId_v1` (with a numeric suffix if that name is taken), chosen consistently across all measurement files, and a warning is emitted. Closes #568. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a285cad commit ce80233

3 files changed

Lines changed: 95 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@
7373
unused ID, and accept switching times that conditions set from parameters
7474
fixed by the parameter table
7575
(by @FFroehlich in https://github.com/PEtab-dev/libpetab-python/pull/574)
76+
* `petab1to2` now renames a pre-existing `experimentId` column in the
77+
measurement table to `experimentId_v1*` instead of a random name, and
78+
warns about it
79+
(by @dweindl in https://github.com/PEtab-dev/libpetab-python/pull/572)
7680

7781
**Features**
7882
* `petab1to2` and `petab_files_1to2` can skip validation via `validate=False`

‎petab/v2/petab1to2.py‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from pathlib import Path
1010
from tempfile import TemporaryDirectory
1111
from urllib.parse import urlparse
12-
from uuid import uuid4
1312

1413
import pandas as pd
1514
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:
227226
v2.get_experiment_df(pd.DataFrame(experiments)), exp_table_path
228227
)
229228

230-
for measurement_file in new_yaml_config.measurement_files:
231-
measurement_df = v1.get_measurement_df(get_src_path(measurement_file))
232-
# if there is already an experiment ID column, we rename it
229+
measurement_dfs = {
230+
measurement_file: v1.get_measurement_df(get_src_path(measurement_file))
231+
for measurement_file in new_yaml_config.measurement_files
232+
}
233+
# if there is already an experiment ID column, we rename it,
234+
# consistently across all measurement tables
235+
if any(
236+
v2.C.EXPERIMENT_ID in df.columns for df in measurement_dfs.values()
237+
):
238+
all_columns = set().union(
239+
*(df.columns for df in measurement_dfs.values())
240+
)
241+
renamed_experiment_id = f"{v2.C.EXPERIMENT_ID}_v1"
242+
i = 1
243+
while renamed_experiment_id in all_columns:
244+
i += 1
245+
renamed_experiment_id = f"{v2.C.EXPERIMENT_ID}_v1_{i}"
246+
warnings.warn(
247+
f"The measurement table has a column `{v2.C.EXPERIMENT_ID}`, "
248+
"which is reserved in PEtab v2. "
249+
f"It was renamed to `{renamed_experiment_id}`.",
250+
# point to the caller of `petab1to2`
251+
stacklevel=3,
252+
)
253+
254+
for measurement_file, measurement_df in measurement_dfs.items():
233255
if v2.C.EXPERIMENT_ID in measurement_df.columns:
234256
measurement_df = measurement_df.rename(
235-
columns={v2.C.EXPERIMENT_ID: f"experiment_id_{uuid4()}"},
257+
columns={v2.C.EXPERIMENT_ID: renamed_experiment_id}
236258
)
237259
# add pre-eq condition id if not present or convert to string
238260
# for simplicity

‎tests/v2/test_conversion.py‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import numpy as np
55
import pandas as pd
66
import pytest
7+
import yaml
78

89
from petab import v1, v2
910
from petab.v1.calculate import calculate_llh as calculate_llh_v1
@@ -612,6 +613,66 @@ def test_petab1to2_validate(tmp_path):
612613
assert len(problem.measurements) == 2
613614

614615

616+
@pytest.mark.parametrize("clash", [False, True])
617+
def test_petab1to2_existing_experiment_id_column(tmp_path, clash):
618+
"""Test that a pre-existing `experimentId` column is renamed
619+
deterministically and consistently across measurement files."""
620+
yaml_file = _write_v1_problem(
621+
tmp_path / "v1",
622+
observables=[
623+
{
624+
v1.C.OBSERVABLE_ID: "obs_B",
625+
v1.C.OBSERVABLE_FORMULA: "B",
626+
v1.C.NOISE_FORMULA: 0.1,
627+
}
628+
],
629+
measurements=[
630+
{
631+
v1.C.OBSERVABLE_ID: "obs_B",
632+
v1.C.SIMULATION_CONDITION_ID: "c0",
633+
v1.C.TIME: 1.0,
634+
v1.C.MEASUREMENT: 0.5,
635+
v2.C.EXPERIMENT_ID: "dose_response",
636+
}
637+
],
638+
parameters=[_K1],
639+
)
640+
# second measurement file, which may have a clashing column
641+
yaml_path = tmp_path / "v1" / yaml_file
642+
yaml_config = yaml.safe_load(yaml_path.read_text())
643+
measurement_df = pd.DataFrame(
644+
{
645+
v1.C.OBSERVABLE_ID: ["obs_B"],
646+
v1.C.SIMULATION_CONDITION_ID: ["c0"],
647+
v1.C.TIME: [2.0],
648+
v1.C.MEASUREMENT: [0.5],
649+
v2.C.EXPERIMENT_ID: ["time_course"],
650+
}
651+
)
652+
if clash:
653+
measurement_df[f"{v2.C.EXPERIMENT_ID}_v1"] = "taken"
654+
v1.write_measurement_df(measurement_df, tmp_path / "v1" / "meas2.tsv")
655+
yaml_config["problems"][0]["measurement_files"].append("meas2.tsv")
656+
yaml_path.write_text(yaml.safe_dump(yaml_config))
657+
658+
expected_name = f"{v2.C.EXPERIMENT_ID}_v1" + ("_2" if clash else "")
659+
with pytest.warns(UserWarning, match=f"renamed to `{expected_name}`"):
660+
problem = petab1to2(yaml_path)
661+
662+
measurement_df = problem.measurement_df
663+
assert measurement_df[expected_name].tolist() == [
664+
"dose_response",
665+
"time_course",
666+
]
667+
assert (
668+
measurement_df[v2.C.EXPERIMENT_ID].tolist() == ["experiment____c0"] * 2
669+
)
670+
if clash:
671+
taken = measurement_df[f"{v2.C.EXPERIMENT_ID}_v1"]
672+
assert pd.isna(taken.iloc[0])
673+
assert taken.iloc[1] == "taken"
674+
675+
615676
try:
616677
import benchmark_models_petab
617678

@@ -634,6 +695,9 @@ def test_petab1to2_validate(tmp_path):
634695
@pytest.mark.filterwarnings(
635696
"ignore:.*does not support log10-transformed observables.*:UserWarning"
636697
)
698+
@pytest.mark.filterwarnings(
699+
"ignore:.*which is reserved in PEtab v2.*:UserWarning"
700+
)
637701
@parametrize_or_skip
638702
def test_benchmark_collection(problem_id):
639703
"""Test that we can upgrade all benchmark collection models."""

0 commit comments

Comments
 (0)