search

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: reduce dependencies and remove metabolite ID suffixes #301

Closed edkerk closed 2 years ago

edkerk commented 2 years ago

Main improvements in this PR:

To reduce dependencies, the model I/O is now completely running on RAVEN, as it previously dependent both on RAVEN and COBRA Toolboxes. All the functions in code/modelTests and code/otherChanges also run solely via RAVEN. loadYeastModel now has a cobra-flag that gives the model in COBRA format instead (default is RAVEN). Not all other code is COBRA-free yet, but this will be done in later PRs when required.

Requires RAVEN 2.7.0.

In addition, the compartment information was removed from the metabolite names and metabolite IDs. This was a remnant of early COBRA-style models, but compartment information is now properly stored elsewhere. Meanwhile, this previous behaviour could cause conflicts, because even if a metabolite's compartment is changed in model.metComps, it would still have the previous compartment information in model.metNames and model.mets. While this will break some backwards compatibility with older code (which can easily be solved on a case-by-case basis), it would be good to get rid of this old baggage.

I hereby confirm that I have:

hongzhonglu commented 2 years ago

Just go through the update, looks great and make life easier! As "I/O is now completely running on RAVEN", it shows COBRA could replace RAVEN if the user did not install RAVEN. I guess the model format is still consistent with cobrapy?

edkerk commented 2 years ago

Yes, it is a valid SBML format, and one can still use COBRA to load the model. The only thing is that if a user is developing yeast-GEM, and therefore making changes to the model that should be pushed back to GitHub, then it is essential to use the build-in saveYeastModel. I tried to explain this here, but I now realize that this might not be completely clear, so I will update this later today: https://github.com/SysBioChalmers/yeast-GEM/blob/2c9e9e823ab029c82d224f638a4aa8e91dceca08/README.md?plain=1#L81-L94.

mihai-sysbio commented 2 years ago

I was trying to go through the changes in yeast-GEM.yml, but because of reordering the diff is impossible to understand. Maybe there's nothing that can be done about this, but then it would great to avoid reordering in future PRs since it makes the work close to untraceable.

edkerk commented 2 years ago

Indeed, this is exactly why sorting was introduced in https://github.com/SysBioChalmers/RAVEN/pull/364, and this will never occur again. This is the first time that sorting is applied to the model, so having large changes now is unavoidable.

I do realize now that sorting the file and removing the metabolic suffices could have been separate PRs to have made things a little more clear.

mihai-sysbio commented 2 years ago

this will never occur again

awesome!