Closed fbergmann closed 2 years ago
Merging #52 (69cf63b) into dev (298f282) will decrease coverage by
3.14%
. The diff coverage is84.88%
.
@@ Coverage Diff @@
## dev #52 +/- ##
==========================================
- Coverage 99.36% 96.21% -3.15%
==========================================
Files 6 6
Lines 313 396 +83
==========================================
+ Hits 311 381 +70
- Misses 2 15 +13
Flag | Coverage Δ | |
---|---|---|
unittests | 96.21% <84.88%> (-3.15%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
biosimulators_copasi/__main__.py | 75.00% <68.75%> (-25.00%) |
:arrow_down: |
biosimulators_copasi/utils.py | 97.79% <92.50%> (-2.21%) |
:arrow_down: |
biosimulators_copasi/core.py | 99.01% <100.00%> (+0.06%) |
:arrow_up: |
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 298f282...69cf63b. Read the comment docs.
This proposes special handling for COPASI-generated SED-ML and OMEX archives. I think this would be best incorporated into COPASi itself. Nevertheless, we can include a utility to standardize files generated by COPASI. I prefer not to directly integrate this into the OMEX/SED-ML execution code. Doing so would give a false impression of valid files. There's already a lot of confusion about these formats, and I don't want to further that. For example, such files would not be executable by other tools or publishable with Biosimulations, as they are aligned with the specifications of the standards.
I integrated this into the archive execution workflow. By default, archives won't automatically be corrected. Archives can be automatically corrected within the archive execution workflow by setting the FIX_COPASI_GENERATED_COMBINE_ARCHIVE
environment variable to 1
. This environment variable can be passed into the Docker image. This environment variable can also be used with the runBioSimulations API. Note, we don't yet provide a way to set such environment variables from the runBioSimulations GUI.
I also added a console entrypoint (fix-copasi-generated-combine-archive
command-line program) for this.
@fbergmann is this similar to what you had in mind? If you agree, I can merge this in and release it.
@jonrkarr indeed this is what i had in mind. I just didn't know how to best hook it in, so that the archive would not have to be extracted twice. The only difference is that my preference would be to have it automatically enabled for the COPASI Biosimulator on run biosimulations.
This is released with 0.1.33. It should be available in runBioSimulations in a few minutes (however, the runBioSimulations UI doesn't provide a way to set the new environment variable).
Because we feel consistency across the community is important, we'd prefer to discuss whether to make this behavior default as a SED-ML community. To ensure that COMBINE archives can be used interchangeably across tools, we feel either (a) no simulation tools should support missing namespaces and media-type format URIs or (b) all tools should be expected to support this. My interpretation is that the SED-ML and OMEX manifest specifications are aligned with the former. For the latter, the SED-ML and OMEX manifest specifications and canonical examples would also need to be revised/clarified.
Currently the COPASI BioSimulator rejects all omex / sed-ml file exported by COPASI. While we can address that in a future version, and export OMEX files that would not trigger those errors. I think it would be great if we could let these things pass here.
The test shows that it would be an easy enough change:
I'm just not sure where this method would need to be called, right at the start of
exec_sedml_docs_in_combine_archive
? Also whether you'd want additional warnings issued.