Skip to content

Conversation

@dweindl
Copy link
Member

@dweindl dweindl commented Apr 28, 2022

No description provided.

@dweindl dweindl requested review from FFroehlich and dilpath April 28, 2022 20:11
@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2022

Codecov Report

Merging #139 (a57419b) into develop (9a49449) will decrease coverage by 0.46%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #139      +/-   ##
===========================================
- Coverage    77.22%   76.75%   -0.47%     
===========================================
  Files           27       27              
  Lines         2955     2956       +1     
  Branches       718      724       +6     
===========================================
- Hits          2282     2269      -13     
- Misses         489      507      +18     
+ Partials       184      180       -4     
Impacted Files Coverage Δ
petab/sbml.py 49.68% <0.00%> (-10.07%) ⬇️
petab/visualize/plotting.py 86.00% <0.00%> (+0.04%) ⬆️
petab/lint.py 67.85% <0.00%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a49449...a57419b. Read the comment docs.

Copy link
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.

Fine to merge as is

sbml.add_global_parameter(model, 'parameter1')
sbml.add_global_parameter(model, 'noiseParameter1_')
sbml.add_global_parameter(model, 'observableParameter1_')
import simplesbml
Copy link
Member

Choose a reason for hiding this comment

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

Move to header? Currently imported multiple times.

Also imported within test cases in other files.

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea was not importing bulky libsbml everywhere, that when running non-sbml only, they will complete slightly faster (e.g., convenient when using auto-rerun).


# compartment missing in model
condition_df['c1'] = [4.0]
condition_df['c2'] = [4.0]
Copy link
Member

Choose a reason for hiding this comment

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

Compartment name changed? Is it that the default compartment in a simplesbml.SbmlModel is already 'c1'?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, exactly.

@dweindl dweindl merged commit 4f5cefc into develop Apr 29, 2022
@dweindl dweindl deleted the simplesbml branch April 29, 2022 08:37
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.

4 participants