Extra error checking and type hinting for FixedElasticSubProblem (#800)

pulp/pulp.py

@@ -689,7 +689,6 @@ class LpAffineExpression(_DICT_TYPE):
        1*x_0 + -3*x_1 + 4*x_2 + 0
     """
 
-    constant: float
     # to remove illegal characters from the names
     trans = maketrans("-+[] ", "_____")
 
@@ -1073,13 +1072,13 @@ def __init__(self, e=None, sense=const.LpConstraintEQ, name=None, rhs=None):
         self.slack = None
         self.modified = True
 
-    def getLb(self):
+    def getLb(self) -> float | None:
         if (self.sense == const.LpConstraintGE) or (self.sense == const.LpConstraintEQ):
             return -self.constant
         else:
             return None
 
-    def getUb(self):
+    def getUb(self) -> float | None:
         if (self.sense == const.LpConstraintLE) or (self.sense == const.LpConstraintEQ):
             return -self.constant
         else:
@@ -1423,7 +1422,7 @@ def __init__(self, name="NoName", sense=const.LpMinimize):
             warnings.warn("Spaces are not permitted in the name. Converted to '_'")
             name = name.replace(" ", "_")
         self.objective: None | LpAffineExpression = None
-        self.constraints = _DICT_TYPE()  # [str, LpConstraint]
+        self.constraints: dict[str, LpConstraint] = _DICT_TYPE()
         self.name = name
         self.sense = sense
         self.sos1 = {}
@@ -1792,7 +1791,15 @@ def __iadd__(self, other):
             )
         return self
 
-    def extend(self, other, use_objective=True):
+    def extend(
+        self,
+        other: (
+            LpProblem
+            | dict[str, LpConstraint]
+            | Iterable[tuple[str, LpConstraint] | LpConstraint]
+        ),
+        use_objective: bool = True,
+    ):
         """
         extends an LpProblem by adding constraints either from a dictionary
         a tuple or another LpProblem object.
@@ -1806,15 +1813,17 @@ def extend(self, other, use_objective=True):
         name
         """
         if isinstance(other, dict):
-            for name in other:
-                self.constraints[name] = other[name]
+            for name, constraint in other.items():
+                self.constraints[name] = constraint
         elif isinstance(other, LpProblem):
             for v in set(other.variables()).difference(self.variables()):
                 v.name = other.name + v.name
             for name, c in other.constraints.items():
                 c.name = other.name + name
                 self.addConstraint(c)
             if use_objective:
+                if other.objective is None:
+                    raise ValueError("Objective not set by provided problem")
                 self.objective += other.objective
         else:
             for c in other:
@@ -2128,26 +2137,24 @@ class FixedElasticSubProblem(LpProblem):
 
     def __init__(
         self,
-        constraint,
-        penalty=None,
-        proportionFreeBound=None,
-        proportionFreeBoundList=None,
+        constraint: LpConstraint,
+        penalty: float | None = None,
+        proportionFreeBound: float | None = None,
+        proportionFreeBoundList: tuple[float, float] | None = None,
     ):
         subProblemName = f"{constraint.name}_elastic_SubProblem"
-        LpProblem.__init__(self, subProblemName, const.LpMinimize)
-        self.objective = LpAffineExpression()
+        super().__init__(subProblemName, const.LpMinimize)
         self.constraint = constraint
         self.constant = constraint.constant
         self.RHS = -constraint.constant
-        self.objective = LpAffineExpression()
         self += constraint, "_Constraint"
         # create and add these variables but disabled
         self.freeVar = LpVariable("_free_bound", upBound=0, lowBound=0)
         self.upVar = LpVariable("_pos_penalty_var", upBound=0, lowBound=0)
         self.lowVar = LpVariable("_neg_penalty_var", upBound=0, lowBound=0)
         constraint.addInPlace(self.freeVar + self.lowVar + self.upVar)
         if proportionFreeBound:
-            proportionFreeBoundList = [proportionFreeBound, proportionFreeBound]
+            proportionFreeBoundList = (proportionFreeBound, proportionFreeBound)
         if proportionFreeBoundList:
             # add a costless variable
             self.freeVar.upBound = abs(constraint.constant * proportionFreeBoundList[0])
@@ -2161,15 +2168,18 @@ def __init__(
             self.upVar.upBound = None
             self.lowVar.lowBound = None
             self.objective = penalty * self.upVar - penalty * self.lowVar
+        else:
+            self.objective = LpAffineExpression()
 
-    def _findValue(self, attrib):
+    def _findValue(self, attrib: str) -> float:
         """
         safe way to get the value of a variable that may not exist
         """
         var = getattr(self, attrib, 0)
         if var:
-            if value(var) is not None:
-                return value(var)
+            val = value(var)
+            if val is not None:
+                return val
             else:
                 return 0.0
         else:
@@ -2191,13 +2201,13 @@ def isViolated(self):
             log.debug(f"isViolated value lhs {self.findLHSValue()} constant {self.RHS}")
         return result
 
-    def findDifferenceFromRHS(self):
+    def findDifferenceFromRHS(self) -> float:
         """
         The amount the actual value varies from the RHS (sense: LHS - RHS)
         """
         return self.findLHSValue() - self.RHS
 
-    def findLHSValue(self):
+    def findLHSValue(self) -> float:
         """
         for elastic constraints finds the LHS value of the constraint without
         the free variable and or penalty variable assumes the constant is on the
@@ -2206,7 +2216,10 @@ def findLHSValue(self):
         upVar = self._findValue("upVar")
         lowVar = self._findValue("lowVar")
         freeVar = self._findValue("freeVar")
-        return self.constraint.value() - self.constant - upVar - lowVar - freeVar
+        constraint = self.constraint.value()
+        if constraint is None:
+            raise ValueError("Constraint has no value")
+        return constraint - self.constant - upVar - lowVar - freeVar
 
     def deElasticize(self):
         """de-elasticize constraint"""
@@ -2222,10 +2235,9 @@ def reElasticize(self):
         self.lowVar.upBound = 0
         self.lowVar.lowBound = None
 
-    def alterName(self, name):
+    def alterName(self, name: str):
         """
         Alters the name of anonymous parts of the problem
-
         """
         self.name = f"{name}_elastic_SubProblem"
         if hasattr(self, "freeVar"):
@@ -2355,7 +2367,7 @@ def isViolated(self):
 
 def lpSum(
     vector: (
-        Iterable[LpAffineExpression]
+        Iterable[LpAffineExpression | LpVariable | int | float]
         | Iterable[tuple[LpElement, float]]
         | int
         | float

pulp/tests/test_pulp.py

@@ -15,6 +15,7 @@
     LpFractionConstraint,
     LpProblem,
     LpVariable,
+    FixedElasticSubProblem,
 )
 from pulp import constants as const
 from pulp import lpSum
@@ -421,7 +422,7 @@ def test_divide(self):
             z = LpVariable("z", 0)
             w = LpVariable("w", 0)
             prob += x + 4 * y + 9 * z, "obj"
-            prob += (2 * x + 2 * y).__div__(2.0) <= 5, "c1"
+            prob += ((2 * x + 2 * y) / 2.0) <= 5, "c1"
             prob += x + z >= 10, "c2"
             prob += -y + z == 7, "c3"
             prob += w >= 0, "c4"
@@ -817,7 +818,21 @@ def test_elastic_constraints_penalty_unbounded(self):
             prob += x + y <= 5, "c1"
             prob += x + z >= 10, "c2"
             prob += -y + z == 7, "c3"
-            prob.extend((w >= -1).makeElasticSubProblem(penalty=0.9))
+
+            sub_prob: FixedElasticSubProblem = (w >= -1).makeElasticSubProblem(
+                penalty=0.9
+            )
+            self.assertEqual(sub_prob.RHS, -1)
+            self.assertEqual(
+                str(sub_prob.objective), "-0.9*_neg_penalty_var + 0.9*_pos_penalty_var"
+            )
+
+            prob.extend(sub_prob)
+
+            elastic_constraint1 = sub_prob.constraints["_Constraint"]
+            elastic_constraint2 = prob.constraints["None_elastic_SubProblem_Constraint"]
+            self.assertEqual(str(elastic_constraint1), str(elastic_constraint2))
+
             if self.solver.__class__ in [
                 COINMP_DLL,
                 GUROBI,
@@ -1303,10 +1318,11 @@ def test_measuring_solving_time(self):
                 delta=delta,
                 msg=f"optimization time for solver {self.solver.name}",
             )
-            self.assertTrue(prob.objective.value() is not None)
+            self.assertIsNotNone(prob.objective)
+            self.assertIsNotNone(prob.objective.value())
             self.assertEqual(status, const.LpStatusOptimal)
             for v in prob.variables():
-                self.assertTrue(v.varValue is not None)
+                self.assertIsNotNone(v.varValue)
 
         @gurobi_test
         def test_time_limit_no_solution(self):