Skip to content

Give PySB import the same reserved-name rename-and-restore as SBML - #3243

Merged
dweindl merged 1 commit into
AMICI-dev:mainfrom
dweindl:fix-pysb-reserved-names
Sep 5, 2026
Merged

Give PySB import the same reserved-name rename-and-restore as SBML#3243
dweindl merged 1 commit into
AMICI-dev:mainfrom
dweindl:fix-pysb-reserved-names

Conversation

@dweindl

@dweindl dweindl commented Sep 4, 2026

Copy link
Copy Markdown
Member

A model quantity literally named one of AMICI's reserved array-parameter
names (t, x, p, k, h, w, y) used to hit a hard ValueError on PySB/BNGL
import, since only SbmlImporter renamed and restored these names before
embedding them anywhere. This applies the same rename-and-restore at
PySB's own symbol-minting sites (parameters, expressions/observables, and
the stoichiometric matrix's derivative computation), sharing the
disambiguation logic with SBML via a new resolve_reserved_symbol_renames
helper.

  • Drops the test_pysb_event "kk" workaround.
  • Drops the empty_compartments_block/motor BNGL expected-failure
    special case -- both now import and simulate successfully.
  • Adds a dedicated regression test (test_pysb_reserved_names).

🤖 Generated with Claude Code

@dweindl dweindl self-assigned this Sep 4, 2026
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.94%. Comparing base (9b17bdd) to head (2a5582a).

Files with missing lines Patch % Lines
python/sdist/amici/importers/utils.py 86.66% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3243      +/-   ##
==========================================
- Coverage   78.32%   77.94%   -0.38%     
==========================================
  Files         318      318              
  Lines       22183    22193      +10     
  Branches     1490     1490              
==========================================
- Hits        17374    17298      -76     
- Misses       4801     4887      +86     
  Partials        8        8              
Flag Coverage Δ
cpp 72.29% <90.90%> (-0.02%) ⬇️
cpp_python 35.03% <87.87%> (+0.04%) ⬆️
petab 48.10% <93.93%> (+0.02%) ⬆️
petab_sciml 16.30% <33.33%> (+0.01%) ⬆️
petab_sciml_benchmarks 14.88% <33.33%> (+0.01%) ⬆️
python 70.74% <90.90%> (-0.02%) ⬇️
sbmlsuite-jax ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
python/sdist/amici/importers/pysb/__init__.py 94.62% <100.00%> (+0.04%) ⬆️
python/sdist/amici/importers/sbml/__init__.py 83.17% <100.00%> (-6.86%) ⬇️
python/sdist/amici/importers/utils.py 83.33% <86.66%> (-1.67%) ⬇️

... and 3 files with indirect coverage changes

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

Model quantities literally named one of AMICI's reserved array-parameter
names (t, x, p, k, h, w, y) used to hit a hard ValueError on PySB/BNGL
import, since only SbmlImporter renamed and restored these before
embedding them. Apply the same rename at PySB's own symbol-minting sites
(parameters, expressions/observables, and the stoichiometric matrix's
derivative computation), sharing the disambiguation logic with SBML via a
new helper.

Drops the test_pysb_event "kk" workaround and the empty_compartments_block/
motor BNGL expected-failure special case, and adds regression coverage.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dweindl
dweindl force-pushed the fix-pysb-reserved-names branch from 460dd97 to 2a5582a Compare September 5, 2026 11:53
@dweindl
dweindl marked this pull request as ready for review September 5, 2026 12:05
@dweindl
dweindl requested a review from a team as a code owner September 5, 2026 12:05
@dweindl
dweindl added this pull request to the merge queue Sep 5, 2026
Merged via the queue into AMICI-dev:main with commit c9b6407 Sep 5, 2026
34 of 35 checks passed
@dweindl
dweindl deleted the fix-pysb-reserved-names branch September 5, 2026 20:34
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.

2 participants