GeomScale / dingo

A python library for metabolic networks sampling and analysis
GNU Lesser General Public License v3.0
42 stars 27 forks source link

Redundant methods in MetabolicNetwork #95

Open vfisikop opened 3 months ago

vfisikop commented 3 months ago

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:

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

hariszaf commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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! :)