Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for specie conversionFactor #259

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

sebapersson
Copy link
Contributor

Closes #257

With this implementation the PEtab.jl SBML importer passes the SBML tests with specie conversion factor. Also, I do not encounter any problem on the other test cases.

Copy link
Collaborator

@exaexa exaexa left a comment

Choose a reason for hiding this comment

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

Looks OK to me, thanks a lot! I'll need to add a writeSBML equivalent, but that should be it. Tests are running now, if formatting fails don't worry about it.

We should also have test coverage -- can you please send me a link (or ideally SBML testcase number :) ) to a model that would require this functionality?

@sebapersson
Copy link
Contributor Author

Thanks! A test case is semantic 00976 can be found here https://github.com/sbmlteam/sbml-test-suite/tree/release/cases/semantic/00976

@sebapersson
Copy link
Contributor Author

@exaexa any status on this?

I could try to also add a test and support for writeSBML, but for the test, I do not really understand the structure of the loadmodels.jl test file

@exaexa
Copy link
Collaborator

exaexa commented Nov 16, 2023

Hi, yes this is in the queue, I will finish the tests and there is no reason for this to not get merged. Sorry for delays, I'm off work these days and don't have much time left for solving stuff.

@exaexa exaexa added the enhancement New feature or request label Nov 16, 2023
@exaexa
Copy link
Collaborator

exaexa commented Nov 16, 2023

@sebapersson I guess we're fixed; I'll release this ASAP once it's merged. You can set your dependencies to SBML v1.5.1 to properly depend on species conversion factors availablility. Thanks again!

Copy link

codecov bot commented Nov 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (62ab21b) 93.28% compared to head (e0a6254) 93.30%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #259      +/-   ##
==========================================
+ Coverage   93.28%   93.30%   +0.01%     
==========================================
  Files          10       10              
  Lines         819      821       +2     
==========================================
+ Hits          764      766       +2     
  Misses         55       55              

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

@exaexa exaexa merged commit 64ec00b into LCSB-BioCore:master Nov 16, 2023
14 checks passed
@exaexa
Copy link
Collaborator

exaexa commented Nov 16, 2023

Ref: JuliaRegistries/General#95464

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for species conversion factor
2 participants