Skip to content

Add Problem.get_unused_id - #561

Merged
dweindl merged 1 commit into
PEtab-dev:mainfrom
dweindl:problem-get-unused-id
Oct 20, 2026
Merged

dweindl merged 1 commit into
PEtab-dev:mainfrom
dweindl:problem-get-unused-id

Conversation

@dweindl

@dweindl dweindl commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Adds petab.v2.Problem.get_unused_id(base, *, exclude=()), which returns base if it is unused, otherwise the first unused base_1, base_2, .... It treats an ID as used if it is any of the following:

  • a PEtab entity ID (parameter, observable, condition, experiment, mapping)
  • a condition target
  • a model entity ID
  • a symbol in an observable or noise formula, a condition target value or a measurement override

assignments_to_experiments now uses it for its new cond_<n> conditions. Before, it only checked condition IDs, so it could create a condition with the ID of, for example, a parameter.

exclude is for choosing several IDs before adding the corresponding entities to the problem.

🤖 Generated with Claude Code

@dweindl dweindl self-assigned this Oct 19, 2026
Add `petab.v2.Problem.get_unused_id`, which returns the given base ID or
the first unused `<base>_<n>`. It checks PEtab entity IDs, condition
targets, symbols in expressions and overrides, and model entity IDs.

Use it in `assignments_to_experiments`, which previously only checked
condition IDs and could create condition IDs that clash with, e.g.,
parameter IDs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dweindl
dweindl force-pushed the problem-get-unused-id branch 2 times, most recently from 84531a2 to eb3c412 Compare October 19, 2026 15:13
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.19%. Comparing base (da74567) to head (eb3c412).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
+ Coverage   78.07%   78.19%   +0.11%     
==========================================
  Files          68       68              
  Lines        7823     7842      +19     
  Branches     1399     1400       +1     
==========================================
+ Hits         6108     6132      +24     
+ Misses       1227     1224       -3     
+ Partials      488      486       -2     

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

@dweindl
dweindl marked this pull request as ready for review October 19, 2026 15:18
@dweindl
dweindl requested a review from a team as a code owner October 19, 2026 15:18

@dilpath dilpath left a comment

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.

Thanks!

Comment thread petab/v2/core.py
ids |= {c.id for c in self.conditions}
ids |= {e.id for e in self.experiments}
ids |= {m.petab_id for m in self.mappings}
ids |= {ch.target_id for c in self.conditions for ch in c.changes}

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.

Is this necessary? If it's to account for SBML model IDs, isn't that already done above, specifically:

or any(m.has_entity_with_id(id_) for m in self.models)

targetId must be a reference in the condition table (I assume), and therefore redundant here.

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.

Technically you are right. If the PEtab problem is valid, this would be covered. If not – for example, if the target ID is not defined in the model yet – a newly added entity with the just checked ID might make it valid and unintentionally cover up the initial error. It's a bit hypothetical, but I guess I'd just leave it in there unless it turns out to be a performance bottleneck.

@dweindl
dweindl merged commit 1307856 into PEtab-dev:main Oct 20, 2026
7 checks passed
@dweindl
dweindl deleted the problem-get-unused-id branch October 20, 2026 13:38
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.

3 participants