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

take another look at addFluxbound #17

Closed bgoli closed 5 years ago

bgoli commented 5 years ago

Taking a look at this, this is really old code. Try consolidate addReaction and addFluxBound to be more flexible.

willigott commented 5 years ago

In this context, I recently ran into the following issue(?):

from cbmpy.CBModel import FluxBound

random_dict = {'a': [1, 2]}

fb = FluxBound(f"{random_dict}_upper_bnd", random_dict, 'lessEqual', 10)

fb.getReactionId()
# returns {'a': [1, 2]}

fb.getId()
# returns "{'a': [1, 2]}_upper_bnd"

Clearly, that was by accident, I actually wanted to use the key and not the dictionary itself. Intuitively, I would say that it should not be allowed to use anything else but strings for IDs.

bgoli commented 5 years ago

This has been fixed in 7627832, model component constructors and now check that str(pid) is a valid id. Standardised the behaviour of classes that redefine setId and those using the base function.