Add support for specie conversionFactor #259
Conversation
There was a problem hiding this comment.
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?
|
Thanks! A test case is semantic 00976 can be found here https://github.com/sbmlteam/sbml-test-suite/tree/release/cases/semantic/00976 |
|
@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 |
|
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. |
|
@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! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
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.