opencobra / cobrapy

COBRApy is a package for constraint-based modeling of metabolic networks.
http://opencobra.github.io/cobrapy/
GNU General Public License v2.0
465 stars 218 forks source link

[Feature] property overload Reaction.lower_bounds/upper_bounds #1403

Open oxinabox opened 2 months ago

oxinabox commented 2 months ago

Checklist

Problem

I want it to be impossible to set a Reaction's lower_bound and upper_bound inconsistently.

That is to say if the lower-bound is ever set to above the current upper-bound I want the upper-bound to also be updated, and visa versa

Solution

If we user property setter overloads we could accomplish this by overloading what happens when you set either of them.

Alternatives

This is the code am currently using

def set_lower_bound(reaction: Reaction, lower_bound: float) -> None:
    if reaction.upper_bound < lower_bound:
        reaction.upper_bound = lower_bound
    reaction.lower_bound = lower_bound

def set_upper_bound(reaction: Reaction, upper_bound: float) -> None:
    if reaction.lower_bound > upper_bound:
        reaction.lower_bound = upper_bound
    reaction.upper_bound = upper_bound

its fine and maybe its the most sensible

Anything else?

This is just an idea i had when I noticed that we were writing code like the above.

But using property overloads like this is pretty magic. It might be too unexpected that changing one thing could result in multiple things changing.

cdiener commented 2 months ago

Hi, thanks for the comment. Lower and upper bounds are already properties and have (overloaded) setters.

The behavior you have in your code used to be the default. But this behavior was deprecated after discussion in favor of using Reaction.bounds = lb, ub which is a bit more explicit.