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

Redundant methods in MetabolicNetwork #95

Open
vfisikop opened this issue Apr 3, 2024 · 3 comments
Open

Redundant methods in MetabolicNetwork #95

vfisikop opened this issue Apr 3, 2024 · 3 comments

Comments

@vfisikop
Copy link
Contributor

vfisikop commented Apr 3, 2024

Starting from here https://github.com/GeomScale/dingo/blob/develop/dingo/MetabolicNetwork.py#L226 it seems that there is a number of methods that are not used anywhere in the code:

  • medium(self, medium: Dict[str, float])
  • set_active_bound(reaction: str, reac_index: int, bound: float)
  • shut_down_reaction(self, index_val)

Is this intentional? Should we remove them or they are going to be used in the future?

hariszaf added a commit to hariszaf/dingo that referenced this issue Apr 25, 2024
hariszaf added a commit to hariszaf/dingo that referenced this issue Apr 25, 2024
@hariszaf
Copy link
Collaborator

Hi @vfisikop

The medium is being used by the user as you can see here.

Though, we were missing a setter, I fixed that already.

I also added an example of how to use this in the README.

Similarly, the shut_down_reaction is supposed not to allow a reaction to occur if the user goes for it.

These functions alter the model, thus the polytope that will be returned later on from the get_polytope .

@vfisikop
Copy link
Contributor Author

Thanks for the clarification. Regarding medium method if it is only used to update the dictionary then why not simply add a method update_medium instead. Then the update will be done in one line of code and (more important) you will not copy the entire dictionary just to update a few entries.

@hariszaf
Copy link
Collaborator

Apologies, I am not sure I am following.
Once the medium is changed we need to change certain things in the model.
That's what we do in the setter

I tried to follow the scheme of cobrapy for that as it's tricky.

I need to still work a little bit on it because now if someone tries this:

for k,v in model.medium.items():
    model.medium[k] = 0
model.medium

will get

{'EX_co2_e': 0, 'EX_glc__D_e': 0, 'EX_h_e': 0, 'EX_h2o_e': 0, 'EX_nh4_e': 0, 'EX_o2_e': 0, 'EX_pi_e': 0}

but if they run model.fba() they still get the initial solution.
Meaning the model.medium looks like it changes but the constraints did not.

If you have any ideas let me know! :)

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

No branches or pull requests

2 participants