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

support of medium modification and cobra model loading #76

Merged
merged 7 commits into from
Jan 25, 2024

Conversation

hariszaf
Copy link
Collaborator

@hariszaf hariszaf commented Sep 9, 2023

Aim of this PR is to address issues #74 and #75 .

The MetabolicNetwork class has been modified now it keeps the exchange reactions of a model as model.exchanges and its medium as model.medium.

The user may modify the medium as in cobra.

On top of that, dingo can also now build a dingo-model based on a cobra-model.
For example:

import cobra 
import dingo 

cobra_model = cobra.io.read_sbml_model("my_model.xml")
dingo_model = dingo.MetabolicNetwork.from_cobra_model(cobra_model)

This allows for further modifications of the model, e.g. adding reactions, through the
convenient interface of cobra and an easier integration with dingo.

@TolisChal
Copy link
Member

TolisChal commented Sep 9, 2023

Hey, @hariszaf, thanks a lot for this PR!

Could you plz add one unit-test per new function to check for correctness?
e.g., load a model that you know the medium and inter-medium reactions and check if the new functions find them correctly.

@hariszaf
Copy link
Collaborator Author

hariszaf commented Sep 9, 2023

@TolisChal here's a test for editing a reaction that is already part of the medium.
The user will be able to edit the medium with reactions that are included in the model.exchanges list.
If they need to add a new reaction, then it would be better to do so using cobra and once they have the model as they want, the can use the dingo.MetabolicNetwork.from_cobra_model().
That's probably something that we should add on the tutorial too.

tests/fba.py Outdated
model.medium = new_media

if model.lb[glc_index] != -1.5:
self.assertTrue(model.lb[glc_index] == -1.5)
Copy link
Member

Choose a reason for hiding this comment

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

How this can be true if model.lb[glc_index] != -1.5?

@@ -40,6 +40,26 @@ def test_fba_sbml(self):

self.assertTrue(abs(res[1] - 0.8739215067486387) < 1e-03)

def test_modify_medium(self):
Copy link
Member

@TolisChal TolisChal Sep 14, 2023

Choose a reason for hiding this comment

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

Does this test that the medium are extracted correctly?
How many medium reaction does ecoli has?
I was thinking something like:
known_medium = {"medium_reaction1": medium_reaction1_index, .... }
and then check if the extracted medium reactions and its indices matches with the already known medium reactions and indices

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean something like that?
The check for the number of exchange reactions is in case in the future we enable to add an exchange reaction, rather than just editing its boundaries.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, exactly. Thanks!

@TolisChal TolisChal merged commit e7a96d9 into GeomScale:develop Jan 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants