SystemsBioinformatics / cbmpy

CBMPy is a Python based platform for constraint based modelling and analysis.
https://systemsbioinformatics.github.io/cbmpy/
GNU General Public License v3.0
19 stars 6 forks source link

setFluxBoundsFromDict can only update all reaction bounds. #26

Open JulLis opened 3 years ago

JulLis commented 3 years ago

If passing a dict with only a subset of all reaction bounds the function fails. If you change the first line from: fbids = self.getFluxBoundIds() to fbids = bounds.keys() you can change all bounds if they are included in the bounds dict or only a subset.

bgoli commented 3 years ago

Thanks for this, you have brought up something interesting here, this function does not actually do what one thinks it does. It treats every fluxbound (lower and upper, so two per reaction) as an individual object and was meant to work with the getAllFluxBounds method.

This is an artifact of supporting the original FBC specifications and now that I look at it, is not so useful. I will add the functionality to update all the reaction bounds from a (partial) dictionary that looks like this:

fbounds = {'rid1' : (lb1, ub1),
           'rid2' : (lb2, ub2),
            ...
}

would that work for you?

In the meantime I suggest iterating over our reaction id's and use this function

 cmod.setReactionBounds(rid, lower, upper)
JulLis commented 3 years ago

Yes, that would be handy. Thanks!

From: Brett Olivier notifications@github.com Sent: Wednesday, November 4, 2020 10:19 AM To: SystemsBioinformatics/cbmpy cbmpy@noreply.github.com Cc: Lischke, J. j.lischke@vu.nl; Author author@noreply.github.com Subject: Re: [SystemsBioinformatics/cbmpy] setFluxBoundsFromDict can only update all reaction bounds. (#26)

Thanks for this, you have brought up something interesting here, this function does not actually do what one thinks it does. It treats every fluxbound (lower and upper, so two per reaction) as an individual object and was meant to work with the getAllFluxBounds method.

This is an artifact of supporting the original FBC specifications and now that I look at it, is not so useful. I will add the functionality to update all the reaction bounds from a (partial) dictionary that looks like this:

fbounds = {'rid1' : (lb1, ub1),

       'rid2' : (lb2, ub2),

        ...

}

would that work for you?

In the meantime I suggest iterating over our reaction id's and use this function

cmod.setReactionBounds(rid, lower, upper)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/SystemsBioinformatics/cbmpy/issues/26#issuecomment-721612864, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AIXJZKM6BZEC2ACMTZBHZDTSOEMATANCNFSM4TJXVCGA.