Skip to content

petab1to2: deterministic name for pre-existing experimentId column - #572

Merged
dweindl merged 4 commits into
PEtab-dev:mainfrom
dweindl:fix_petab1to2_experiment_id
Oct 26, 2026
Merged

dweindl merged 4 commits into
PEtab-dev:mainfrom
dweindl:fix_petab1to2_experiment_id

Conversation

@dweindl

@dweindl dweindl commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

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

@dweindl
dweindl marked this pull request as ready for review October 25, 2026 08:47
@dweindl
dweindl requested a review from a team as a code owner October 25, 2026 08:47
@dweindl
dweindl force-pushed the fix_petab1to2_experiment_id branch from 6b1d149 to e7319f2 Compare October 25, 2026 13:12
@codecov-commenter

codecov-commenter commented Oct 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.06%. Comparing base (a285cad) to head (4559969).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #572      +/-   ##
==========================================
+ Coverage   79.04%   79.06%   +0.01%     
==========================================
  Files          70       70              
  Lines        8157     8164       +7     
  Branches     1461     1463       +2     
==========================================
+ Hits         6448     6455       +7     
  Misses       1220     1220              
  Partials      489      489              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread petab/v2/petab1to2.py Outdated
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}`.",
# call to `petab1to2`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refers to the stacklevel. The problem is that we have potentially different user entry points. Currently, the stacklevel assumes that a user called petab1to2. Clarified.

Comment thread CHANGELOG.md Resolved
dweindl and others added 3 commits October 26, 2026 11:47
Rename a pre-existing `experimentId` column in v1 measurement tables to
`experimentId_v1` (with a numeric suffix if taken) instead of
`experiment_id_<uuid4>`, consistently across all measurement files, and
warn about the rename.

Closes PEtab-dev#568.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dweindl
dweindl force-pushed the fix_petab1to2_experiment_id branch from cdd1b56 to 52311cb Compare October 26, 2026 10:48
@dweindl
dweindl enabled auto-merge (squash) October 26, 2026 11:06
@dweindl
dweindl merged commit ce80233 into PEtab-dev:main Oct 26, 2026
7 checks passed
@dweindl
dweindl deleted the fix_petab1to2_experiment_id branch October 26, 2026 11:17
dweindl added a commit to AMICI-dev/AMICI that referenced this pull request Oct 26, 2026
…3494)

Since PEtab-dev/libpetab-python#572, `petab1to2` warns when it renames a
v1 measurement table column `experimentId`, which is reserved in PEtab
v2. With warnings as errors, the v1→v2 auto-upgrade fails, and so do the
JAX benchmark tests for Brannmark_JBC2010 and Isensee_JCB2018 (e.g.,
https://github.com/AMICI-dev/AMICI/actions/runs/40377463587/job/121764332167).
The renamed column is not used for simulation, so the warning is
ignored.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

petab1to2: random column name for pre-existing experimentId column

3 participants