Merge pull request #206 from jonls/lpsolver-redefine-error

lpsolver: Raise error when redefining variables

psamm/lpsolver/cplex.py

@@ -128,16 +128,21 @@ def cplex(self):
         return self._cp
 
     def define(self, *names, **kwargs):
-        """Define variable in the problem.
+        """Define a variable in the problem.
 
         Variables must be defined before they can be accessed by var() or
         set(). This function takes keyword arguments lower and upper to define
         the bounds of the variable (default: -inf to inf). The keyword argument
         types can be used to select the type of the variable (Continuous
         (default), Binary or Integer). Setting any variables different than
-        Continuous will turn the problem into an MILP problem.
+        Continuous will turn the problem into an MILP problem. Raises
+        ValueError if a name is already defined.
         """
         names = tuple(names)
+        for name in names:
+            if name in self._variables:
+                raise ValueError('Variable already defined: {!r}'.format(name))
+
         lower = kwargs.get('lower', None)
         upper = kwargs.get('upper', None)
         vartype = kwargs.get('types', None)

psamm/lpsolver/glpk.py

@@ -112,16 +112,21 @@ def glpk(self):
         return self._p
 
     def define(self, *names, **kwargs):
-        """Define variable in the problem.
+        """Define a variable in the problem.
 
         Variables must be defined before they can be accessed by var() or
         set(). This function takes keyword arguments lower and upper to define
         the bounds of the variable (default: -inf to inf). The keyword argument
         types can be used to select the type of the variable (Continuous
         (default), Binary or Integer). Setting any variables different than
-        Continuous will turn the problem into an MILP problem.
+        Continuous will turn the problem into an MILP problem. Raises
+        ValueError if a name is already defined.
         """
         names = tuple(names)
+        for name in names:
+            if name in self._variables:
+                raise ValueError('Variable already defined: {!r}'.format(name))
+
         lower = kwargs.get('lower', None)
         upper = kwargs.get('upper', None)
         vartype = kwargs.get('types', None)

psamm/lpsolver/gurobi.py

@@ -104,16 +104,21 @@ def gurobi(self):
         return self._p
 
     def define(self, *names, **kwargs):
-        """Define variable in the problem.
+        """Define a variable in the problem.
 
         Variables must be defined before they can be accessed by var() or
         set(). This function takes keyword arguments lower and upper to define
         the bounds of the variable (default: -inf to inf). The keyword argument
         types can be used to select the type of the variable (Continuous
         (default), Binary or Integer). Setting any variables different than
-        Continuous will turn the problem into an MILP problem.
+        Continuous will turn the problem into an MILP problem. Raises
+        ValueError if a name is already defined.
         """
         names = tuple(names)
+        for name in names:
+            if name in self._variables:
+                raise ValueError('Variable already defined: {!r}'.format(name))
+
         lower = kwargs.get('lower', None)
         upper = kwargs.get('upper', None)
         vartype = kwargs.get('types', None)

psamm/lpsolver/lp.py

@@ -545,6 +545,9 @@ def define(self, names, **kwargs):
         self._problem.define(
             *((self, name) for name in names), **define_kwargs)
 
+    def has_variable(self, name):
+        return self._problem.has_variable((self, name))
+
     def __call__(self, name):
         return self._problem.var((self, name))
 
@@ -576,7 +579,16 @@ class Problem(object):
 
     @abc.abstractmethod
     def define(self, *names, **kwargs):
-        """Define a variable in the problem"""
+        """Define a variable in the problem.
+
+        Variables must be defined before they can be accessed by var() or
+        set(). This function takes keyword arguments lower and upper to define
+        the bounds of the variable (default: -inf to inf). The keyword argument
+        types can be used to select the type of the variable (Continuous
+        (default), Binary or Integer). Setting any variables different than
+        Continuous will turn the problem into an MILP problem. Raises
+        ValueError if a name is already defined.
+        """
 
     @abc.abstractmethod
     def has_variable(self, name):

psamm/lpsolver/qsoptex.py

@@ -67,15 +67,19 @@ def qsoptex(self):
         return self._p
 
     def define(self, *names, **kwargs):
-        """Define variable in the problem
+        """Define a variable in the problem.
 
         Variables must be defined before they can be accessed by var() or
         set(). This function takes keyword arguments lower and upper to define
         the bounds of the variable (default: -inf to inf). The keyword argument
-        types can be used to select the type of the variable (Only Continuous
-        is suported).
+        types can be used to select the type of the variable (only Continuous
+        is supported). Raises ValueError if a name is already defined.
         """
         names = tuple(names)
+        for name in names:
+            if name in self._variables:
+                raise ValueError('Variable already defined: {!r}'.format(name))
+
         lower = kwargs.get('lower', None)
         upper = kwargs.get('upper', None)
         vartype = kwargs.get('types', None)

psamm/tests/test_lpsolver_generic.py

@@ -22,6 +22,33 @@
 from psamm.lpsolver import generic
 
 
+class TestSolverProblem(unittest.TestCase):
+    """Test the current solver chosen by generic.
+
+    To test all solvers, run this test multiple times with the PSAMM_SOLVER
+    environment variable set to the solver to test.
+    """
+    def setUp(self):
+        try:
+            self.solver = generic.Solver()
+        except generic.RequirementsError:
+            self.skipTest('Unable to find an LP solver for tests')
+
+    def test_redefine_variable(self):
+        """Test that redefining a variable fails."""
+        prob = self.solver.create_problem()
+        prob.define('x', lower=3, upper=100)
+        with self.assertRaises(ValueError):
+            prob.define('x', lower=3, upper=100)
+
+    def test_has_variable(self):
+        """Test whether has_variable() works."""
+        prob = self.solver.create_problem()
+        self.assertFalse(prob.has_variable('x'))
+        prob.define('x', lower=-400)
+        self.assertTrue(prob.has_variable('x'))
+
+
 class TestListSolversCommand(unittest.TestCase):
     def test_list_lpsolvers(self):
         if os.environ.get('PSAMM_SOLVER') in ('nosolver', None):