Skip to content

Minor improvement in v1 to v2 converter#479

Open
m-philipps wants to merge 6 commits intomainfrom
minor_fix
Open

Minor improvement in v1 to v2 converter#479
m-philipps wants to merge 6 commits intomainfrom
minor_fix

Conversation

@m-philipps
Copy link
Copy Markdown
Collaborator

@m-philipps m-philipps commented May 8, 2026

  • add Warning when removing PARAMETER_SCALE, INITIALIZATION_PRIOR_TYPE, INITIALIZATION_PRIOR_PARAMETERS
  • Use the column order for the experiments table that is suggested in the PEtab format
  • Fix description and redundancy in v2 get_experiment_df

@m-philipps m-philipps requested a review from a team as a code owner May 8, 2026 14:30
@m-philipps m-philipps changed the title Minor improvement in v1 t0 v2 converter Minor improvement in v1 to v2 converter May 8, 2026
@m-philipps m-philipps marked this pull request as draft May 8, 2026 15:05
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.43%. Comparing base (f8ba6b6) to head (a59dab9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #479   +/-   ##
=======================================
  Coverage   75.43%   75.43%           
=======================================
  Files          62       62           
  Lines        6899     6901    +2     
  Branches     1228     1229    +1     
=======================================
+ Hits         5204     5206    +2     
  Misses       1223     1223           
  Partials      472      472           

☔ View full report in Codecov by Sentry.
📢 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.

@m-philipps m-philipps marked this pull request as ready for review May 8, 2026 15:44
Copy link
Copy Markdown
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks

Comment thread petab/v2/petab1to2.py
)
# some columns were dropped in PEtab v2
if v1.C.INITIALIZATION_PRIOR_TYPE in df and (
df[v1.C.INITIALIZATION_PRIOR_TYPE].notna().any()
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.

Fine for me to drop the notna check and warn regardless.

Comment thread petab/v2/petab1to2.py
Comment on lines +534 to +538
if not (df[v1.C.PARAMETER_SCALE] == v1.C.LIN).all():
warnings.warn(
"Parameter scales are not supported in PEtab v2.",
stacklevel=9,
)
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.

Same here, fine for me to warn if the column is present at all

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