fix #657: reverse inplace operations after leaving context (#666)

cobra/core/reaction.py

@@ -420,7 +420,7 @@ def gene_reaction_rule(self, new_rule):
             if g not in self._genes:  # if an old gene is not a new gene
                 try:
                     g._reaction.remove(self)
-                except:
+                except KeyError:
                     warn("could not remove old gene %s from reaction %s" %
                          (g.id, self.id))
 
@@ -618,6 +618,7 @@ def __add__(self, other):
     __radd__ = __add__
 
     def __iadd__(self, other):
+
         self.add_metabolites(other._metabolites, combine=True)
         gpr1 = self.gene_reaction_rule.strip()
         gpr2 = other.gene_reaction_rule.strip()
@@ -643,11 +644,26 @@ def __isub__(self, other):
     def __imul__(self, coefficient):
         """Scale coefficients in a reaction by a given value
 
-        E.g. A -> B becomes 2A -> 2B
+        E.g. A -> B becomes 2A -> 2B.
+
+        If coefficient is less than zero, the reaction is reversed and the
+        bounds are swapped.
         """
         self._metabolites = {
             met: value * coefficient
             for met, value in iteritems(self._metabolites)}
+
+        if coefficient < 0:
+            self.bounds = (-self.upper_bound, -self.lower_bound)
+
+        if self._model:
+            self._model._populate_solver([self])
+
+        context = get_context(self)
+        if context:
+            context(partial(self._model._populate_solver, [self]))
+            context(partial(self.__imul__, 1./coefficient))
+
         return self
 
     def __mul__(self, coefficient):
@@ -757,37 +773,25 @@ def add_metabolites(self, metabolites_to_add, combine=True,
                 # reaction
                 metabolite._reaction.add(self)
 
-        for metabolite, the_coefficient in list(self._metabolites.items()):
-            if the_coefficient == 0:
-                # make the metabolite aware that it no longer participates
-                # in this reaction
-                metabolite._reaction.remove(self)
-                self._metabolites.pop(metabolite)
-
         # from cameo ...
         model = self.model
         if model is not None:
             model.add_metabolites(new_metabolites)
 
-            for metabolite, coefficient in metabolites_to_add.items():
-
-                if isinstance(metabolite,
-                              str):  # support metabolites added as strings.
-                    metabolite = model.metabolites.get_by_id(metabolite)
-                if combine:
-                    try:
-                        old_coefficient = old_coefficients[metabolite]
-                    except KeyError:
-                        pass
-                    else:
-                        coefficient = coefficient + old_coefficient
-
+            for metabolite, coefficient in self._metabolites.items():
                 model.constraints[
                     metabolite.id].set_linear_coefficients(
                     {self.forward_variable: coefficient,
                      self.reverse_variable: -coefficient
                      })
 
+        for metabolite, the_coefficient in list(self._metabolites.items()):
+            if the_coefficient == 0:
+                # make the metabolite aware that it no longer participates
+                # in this reaction
+                metabolite._reaction.remove(self)
+                self._metabolites.pop(metabolite)
+
         context = get_context(self)
         if context and reversibly:
             if combine:

cobra/test/test_solver_model.py

@@ -463,6 +463,26 @@ def test_add_metabolites_combine_false(self, model):
                 already_included_metabolite.id].expression.has(
                 -10 * reaction.reverse_variable)
 
+    def test_reaction_imul(self, model):
+        with model:
+            model.reactions.EX_glc__D_e *= 100
+            assert model.constraints.glc__D_e.expression.coeff(
+                model.variables.EX_glc__D_e) == -100
+            assert model.reactions.EX_glc__D_e.reaction == \
+                '100.0 glc__D_e <=> '
+
+        assert model.constraints.glc__D_e.expression.coeff(
+            model.variables.EX_glc__D_e) == -1
+        assert model.reactions.EX_glc__D_e.reaction == 'glc__D_e <=> '
+
+        with model:
+            model.reactions.EX_glc__D_e *= -2
+            assert model.reactions.EX_glc__D_e.bounds == (-1000.0, 10.0)
+            assert model.reactions.EX_glc__D_e.reaction == ' <=> 2.0 glc__D_e'
+
+        assert model.reactions.EX_glc__D_e.bounds == (-10, 1000.0)
+        assert model.reactions.EX_glc__D_e.reaction == 'glc__D_e <=> '
+
     # def test_pop(self, model):
     #     pgi = model.reactions.PGI
     #     g6p = model.metabolites.get_by_id("g6p_c")