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

Update to SBML 5.16 #1054

Merged
merged 13 commits into from
Dec 12, 2017
Merged

Update to SBML 5.16 #1054

merged 13 commits into from
Dec 12, 2017

Conversation

tpfau
Copy link
Contributor

@tpfau tpfau commented Dec 11, 2017

Update to SBML 5.16
This update includes:
Corrected IO for subSystems (they are now exported via the groups plugin - in accordance with BiGG models). They can now also be read from those.
Fixed issues with macOS high Sierra/Sierra and current Matlab versions.
SBML IO should work now with all current Matlab/macOS combinations indicated by mathworks.

I hereby confirm that I have:

  • Tested my code on my own machine
  • Followed the guidelines in the Contributing Guide
  • Selected develop as a target branch (top left drop-down menu)

(Note: You may replace [] with [X] to check the box)

@codecov-io
Copy link

codecov-io commented Dec 11, 2017

Codecov Report

Merging #1054 into develop will increase coverage by 0.04%.
The diff coverage is 72.72%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1054      +/-   ##
===========================================
+ Coverage    41.76%   41.81%   +0.04%     
===========================================
  Files          809      764      -45     
  Lines        59216    56480    -2736     
===========================================
- Hits         24730    23615    -1115     
+ Misses       34486    32865    -1621
Impacted Files Coverage Δ
src/base/install/adaptMacPath.m 0% <0%> (ø)
.../base/io/utilities/SBML/makeSBMLAnnotationString.m 85.71% <100%> (ø) ⬆️
src/analysis/exploration/getModelSubSystems.m 83.33% <100%> (+8.33%) ⬆️
src/base/io/utilities/readSBML.m 91.47% <100%> (+0.6%) ⬆️
src/base/io/writeCbModel.m 49.1% <7.69%> (-0.9%) ⬇️
src/base/io/utilities/writeSBML.m 86.73% <94.93%> (+9.61%) ⬆️
...alance/basicPhysicochemicalData/getMolecularMass.m 0% <0%> (-100%) ⬇️
...c/visualization/maps/ReconMap/postMINERVArequest.m 0% <0%> (-92.86%) ⬇️
.../visualization/maps/ReconMap/buildFluxDistLayout.m 0% <0%> (-92.31%) ⬇️
...rc/reconstruction/modelGeneration/removeDeadEnds.m 0% <0%> (-91.67%) ⬇️
... and 69 more

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 47fe1fa...3324ebc. Read the comment docs.

Copy link
Contributor

@laurentheirendt laurentheirendt left a comment

Choose a reason for hiding this comment

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

May you change the permissions to 644?

else
if strcmp(format,'toselect') % no format was given. try to detect from fileName.
[~, ~, extension] = fileparts(fileName);
switch extension
Copy link
Contributor

Choose a reason for hiding this comment

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

May you put this switch in an internal function?

end


function adaptMacPath()
Copy link
Contributor

Choose a reason for hiding this comment

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

May you put this function as a separate function in base?

@tpfau
Copy link
Contributor Author

tpfau commented Dec 11, 2017

Thanks for the comments. Is this what you wanted?

@tpfau
Copy link
Contributor Author

tpfau commented Dec 11, 2017

So all tests pass, but codecov fails and thus the default?

@tpfau
Copy link
Contributor Author

tpfau commented Dec 12, 2017

Close/Reopen to retrigger the CI

@tpfau tpfau closed this Dec 12, 2017
@tpfau tpfau reopened this Dec 12, 2017
@rmtfleming rmtfleming merged commit bac5d4b into opencobra:develop Dec 12, 2017
@tpfau tpfau deleted the SBMLGroupsUpdate branch December 14, 2017 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants