fix: add metabolites in nested contexts (#466)

With model as context, reaction.add_metabolites would make an
addition of a partial call to reaction.subtract_metabolites, which in
turn called reaction.add_metabolites thus adding the revert itself back
to the history stack. Avoid this by adding a flag to avoid this addition
from happening.

Also add small performance boost to not go through with
model.add_metabolites on empty list (can happen fairly often when
manipulating many reactions).

cobra/core/model.py

@@ -331,6 +331,9 @@ def add_metabolites(self, metabolite_list):
         """
         if not hasattr(metabolite_list, '__iter__'):
             metabolite_list = [metabolite_list]
+        if len(metabolite_list) == 0:
+            return None
+
         # First check whether the metabolites exist in the model
         metabolite_list = [x for x in metabolite_list
                            if x.id not in self.metabolites]

cobra/core/reaction.py

@@ -686,7 +686,8 @@ def get_coefficients(self, metabolite_ids):
         """
         return map(self.get_coefficient, metabolite_ids)
 
-    def add_metabolites(self, metabolites_to_add, combine=True):
+    def add_metabolites(self, metabolites_to_add, combine=True,
+                        reversibly=True):
         """Add metabolites and stoichiometric coefficients to the reaction.
         If the final coefficient for a metabolite is 0 then it is removed
         from the reaction.
@@ -696,13 +697,19 @@ def add_metabolites(self, metabolites_to_add, combine=True):
         Parameters
         ----------
         metabolites_to_add : dict
-            {str or :class:`~cobra.core.Metabolite.Metabolite`: coefficient}
+            Dictionary with metabolite objects or metabolite identifiers as
+            keys and coefficients as values.
+
 
         combine : bool
             Describes behavior a metabolite already exists in the reaction.
             True causes the coefficients to be added.
             False causes the coefficient to be replaced.
 
+        reversibly : bool
+            Whether to add the change to the context to make the change
+            reversibly or not (primarily intended for internal use).
+
         """
         old_coefficients = self.metabolites
         new_metabolites = []
@@ -772,12 +779,13 @@ def add_metabolites(self, metabolites_to_add, combine=True):
                      })
 
         context = get_context(self)
-        if context:
-            # Just subtact the metabolites that were added
+        if context and reversibly:
+            # Just subtract the metabolites that were added
             context(partial(
-                self.subtract_metabolites, metabolites_to_add, combine=True))
+                self.subtract_metabolites, metabolites_to_add, combine=True,
+                reversibly=False))
 
-    def subtract_metabolites(self, metabolites, combine=True):
+    def subtract_metabolites(self, metabolites, combine=True, reversibly=True):
         """This function will 'subtract' metabolites from a reaction, which
         means add the metabolites with -1*coefficient. If the final coefficient
         for a metabolite is 0 then the metabolite is removed from the reaction.
@@ -796,11 +804,15 @@ def subtract_metabolites(self, metabolites, combine=True):
             True causes the coefficients to be added.
             False causes the coefficient to be replaced.
 
+        reversibly : bool
+            Whether to add the change to the context to make the change
+            reversibly or not (primarily intended for internal use).
+
         .. note:: A final coefficient < 0 implies a reactant.
 
         """
         self.add_metabolites({k: -v for k, v in iteritems(metabolites)},
-                             combine=combine)
+                             combine=combine, reversibly=reversibly)
 
     @property
     def reaction(self):

cobra/test/test_model.py

@@ -92,16 +92,17 @@ def add_remove_metabolite():
         benchmark(add_remove_metabolite)
 
     def test_add_metabolite(self, model):
-
         with model:
-            reaction = model.reactions.get_by_id("PGI")
-            reaction.add_metabolites({model.metabolites[0]: 1})
-            assert model.metabolites[0] in reaction._metabolites
-            fake_metabolite = Metabolite("fake")
-            reaction.add_metabolites({fake_metabolite: 1})
-            assert fake_metabolite in reaction._metabolites
-            assert model.metabolites.has_id("fake")
-            assert model.metabolites.get_by_id("fake") is fake_metabolite
+            with model:
+                reaction = model.reactions.get_by_id("PGI")
+                reaction.add_metabolites({model.metabolites[0]: 1})
+                assert model.metabolites[0] in reaction._metabolites
+                fake_metabolite = Metabolite("fake")
+                reaction.add_metabolites({fake_metabolite: 1})
+                assert fake_metabolite in reaction._metabolites
+                assert model.metabolites.has_id("fake")
+                assert model.metabolites.get_by_id("fake") is fake_metabolite
+            assert len(model._contexts[0]._history) == 0
 
         assert fake_metabolite._model is None
         assert fake_metabolite not in reaction._metabolites