SysBioChalmers / yeast-GEM

The consensus GEM for Saccharomyces cerevisiae
http://sysbiochalmers.github.io/yeast-GEM/
Creative Commons Attribution 4.0 International
96 stars 46 forks source link

refactor: standard-GEM #257

Closed edkerk closed 3 years ago

edkerk commented 3 years ago

Main improvements in this PR:

I hereby confirm that I have:

edkerk commented 3 years ago

Awesome!

Looking at the /model/* files, I see they are named yeastGEM instead of the expected yeast-GEM. Any thoughts on the standard-GEM indication that

All model files must be named the same as the repository

?

I don't see a reason why not to adhere to the standard-GEM indication on this, so seems fine to correct this.

edkerk commented 3 years ago

Everything looks good, except there is deletion of one entry of model xml file, if that is ok, then everything is fine.

@feiranl This is an artifact of the empty subSystems. I've now manually removed them (model=rmfield(model,'subSystems')) before running saveYeastModel. This will be fully fixed when subSystems are added (see #11).

mihai-sysbio commented 3 years ago

@edkerk I'm curious, why was 0ef8f3f necessary?

edkerk commented 3 years ago

Just for reference, coming back to naming of the model files. In commit 919d396, I also changed the model.id field from yeastGEM to yeast-GEM. However, - is not allowed in this ID in SBML, so that it either would end up with an invalid SBML file, or it would end up with a model.id that reads yeast__45__GEM if replaced with ASCII code as COBRA routinely does. This also raises issues when the model is then loaded with RAVEN or cobrapy instead as these do not automatically modify the IDs to replace ASCII codes. So best to prevent all of this, while standard-GEM also does not specify that the model.id should have a certain format, just the model filename. This is now fixed with 0ef8f3f and aab4e8d.

edkerk commented 3 years ago

@mihai-sysbio Very good point, the SBML documentation is very clear on what is allowed, so it would just involve a few regex-checks.