Streamline redundant parameter passing in constraint constructors

spopt/locate/coverage.py

@@ -181,8 +181,9 @@ def from_cost_matrix(
 
         """
 
+        n_cli = cost_matrix.shape[0]
+        r_cli = range(n_cli)
         r_fac = range(cost_matrix.shape[1])
-        r_cli = range(cost_matrix.shape[0])
 
         model = pulp.LpProblem(name, pulp.LpMinimize)
         lscp = LSCP(name, model)
@@ -198,13 +199,13 @@ def from_cost_matrix(
         lscp.aij[cost_matrix <= service_radius] = 1
 
         if (demand_quantity_arr is None) and (facility_capacity_arr is not None):
-            demand_quantity_arr = np.ones(cost_matrix.shape[0])
+            demand_quantity_arr = np.ones(n_cli)
 
         FacilityModelBuilder.add_facility_integer_variable(lscp, r_fac, "y[{i}]")
 
         if predefined_facilities_arr is not None:
             FacilityModelBuilder.add_predefined_facility_constraint(
-                lscp, lscp.problem, predefined_facilities_arr
+                lscp, predefined_facilities_arr
             )
 
         if demand_quantity_arr is not None:
@@ -222,22 +223,15 @@ def from_cost_matrix(
             )
 
             FacilityModelBuilder.add_facility_capacity_constraint(
-                lscp,
-                lscp.problem,
-                facility_capacity_arr,
-                demand_quantity_arr,
-                r_fac,
-                r_cli,
+                lscp, demand_quantity_arr, facility_capacity_arr, r_cli, r_fac
             )
 
             FacilityModelBuilder.add_client_demand_satisfaction_constraint(
-                lscp, lscp.problem, lscp.aij, r_cli, r_fac
+                lscp, r_cli, r_fac
             )
 
         else:
-            FacilityModelBuilder.add_set_covering_constraint(
-                lscp, lscp.problem, lscp.aij, r_fac, r_cli
-            )
+            FacilityModelBuilder.add_set_covering_constraint(lscp, r_cli, r_fac)
 
         lscp.__add_obj()
 
@@ -432,7 +426,7 @@ def facility_client_array(self) -> None:
             array_cli = []
             if fac_vars[j].value() > 0:
                 for i in range(self.aij.shape[0]):
-                    if self.aij[i][j] > 0:
+                    if self.aij[i, j] > 0:
                         array_cli.append(i)
 
             self.fac2cli.append(array_cli)
@@ -666,8 +660,8 @@ def from_cost_matrix(
             lscp = LSCP.from_cost_matrix(cost_matrix, service_radius)
         lscp.solve(solver)
 
-        r_fac = range(cost_matrix.shape[1])
         r_cli = range(cost_matrix.shape[0])
+        r_fac = range(cost_matrix.shape[1])
 
         model = pulp.LpProblem(name, pulp.LpMaximize)
 
@@ -682,16 +676,12 @@ def from_cost_matrix(
 
         if predefined_facilities_arr is not None:
             FacilityModelBuilder.add_predefined_facility_constraint(
-                lscpb, lscpb.problem, predefined_facilities_arr
+                lscpb, predefined_facilities_arr
             )
 
         lscpb.__add_obj()
-        FacilityModelBuilder.add_facility_constraint(
-            lscpb, lscpb.problem, lscpb.lscp_obj_value
-        )
-        FacilityModelBuilder.add_backup_covering_constraint(
-            lscpb, lscpb.problem, lscpb.aij, r_fac, r_cli
-        )
+        FacilityModelBuilder.add_facility_constraint(lscpb, lscpb.lscp_obj_value)
+        FacilityModelBuilder.add_backup_covering_constraint(lscpb, r_fac, r_cli)
 
         return lscpb
 
@@ -874,7 +864,7 @@ def facility_client_array(self) -> None:
             array_cli = []
             if fac_vars[j].value() > 0:
                 for i in range(self.aij.shape[0]):
-                    if self.aij[i][j] > 0:
+                    if self.aij[i, j] > 0:
                         array_cli.append(i)
 
             self.fac2cli.append(array_cli)
@@ -1101,8 +1091,10 @@ def from_cost_matrix(
         99.0
 
         """
+
+        n_cli = cost_matrix.shape[0]
+        r_cli = range(n_cli)
         r_fac = range(cost_matrix.shape[1])
-        r_cli = range(cost_matrix.shape[0])
 
         model = pulp.LpProblem(name, pulp.LpMaximize)
         mclp = MCLP(name, model)
@@ -1112,20 +1104,18 @@ def from_cost_matrix(
 
         mclp.aij = np.zeros(cost_matrix.shape)
         mclp.aij[cost_matrix <= service_radius] = 1
-        weights = np.reshape(weights, (cost_matrix.shape[0], 1))
+        weights = np.reshape(weights, (n_cli, 1))
 
         mclp.__add_obj(weights, r_cli)
 
         if predefined_facilities_arr is not None:
             FacilityModelBuilder.add_predefined_facility_constraint(
-                mclp, mclp.problem, predefined_facilities_arr
+                mclp, predefined_facilities_arr
             )
 
-        FacilityModelBuilder.add_maximal_coverage_constraint(
-            mclp, mclp.problem, mclp.aij, r_fac, r_cli
-        )
+        FacilityModelBuilder.add_maximal_coverage_constraint(mclp, r_fac, r_cli)
 
-        FacilityModelBuilder.add_facility_constraint(mclp, mclp.problem, p_facilities)
+        FacilityModelBuilder.add_facility_constraint(mclp, p_facilities)
 
         return mclp
 
@@ -1327,7 +1317,7 @@ def facility_client_array(self) -> None:
             if fac_vars[j].value() > 0:
                 for i in range(self.aij.shape[0]):
                     if cli_vars[i].value() > 0:
-                        if self.aij[i][j] > 0:
+                        if self.aij[i, j] > 0:
                             array_cli.append(i)
 
             self.fac2cli.append(array_cli)

spopt/locate/p_center.py

@@ -200,22 +200,16 @@ def from_cost_matrix(
 
         if predefined_facilities_arr is not None:
             FacilityModelBuilder.add_predefined_facility_constraint(
-                p_center, p_center.problem, predefined_facilities_arr
+                p_center, predefined_facilities_arr
             )
 
         p_center.__add_obj()
 
-        FacilityModelBuilder.add_facility_constraint(
-            p_center, p_center.problem, p_facilities
-        )
-        FacilityModelBuilder.add_assignment_constraint(
-            p_center, p_center.problem, r_fac, r_cli
-        )
-        FacilityModelBuilder.add_opening_constraint(
-            p_center, p_center.problem, r_fac, r_cli
-        )
+        FacilityModelBuilder.add_facility_constraint(p_center, p_facilities)
+        FacilityModelBuilder.add_assignment_constraint(p_center, r_fac, r_cli)
+        FacilityModelBuilder.add_opening_constraint(p_center, r_fac, r_cli)
         FacilityModelBuilder.add_minimized_maximum_constraint(
-            p_center, p_center.problem, cost_matrix, r_fac, r_cli
+            p_center, cost_matrix, r_fac, r_cli
         )
 
         return p_center
@@ -388,7 +382,7 @@ def facility_client_array(self) -> None:
             array_cli = []
             if fac_vars[j].value() > 0:
                 for i in range(len(cli_vars)):
-                    if cli_vars[i][j].value() > 0:
+                    if cli_vars[i, j].value() > 0:
                         array_cli.append(i)
 
             self.fac2cli.append(array_cli)

spopt/locate/p_dispersion.py

@@ -88,13 +88,13 @@ def from_cost_matrix(
         Parameters
         ----------
 
-        cost_matrix: np.array
+        cost_matrix : np.array
             A cost matrix in the form of a 2D array between origins and destinations.
         p_facilities : int
             The number of facilities to be located.
         predefined_facilities_arr : numpy.array (default None)
             Predefined facilities that must appear in the solution.
-        name: str (default 'P-Dispersion')
+        name : str (default 'P-Dispersion')
             The name of the problem.
 
         Returns
@@ -173,16 +173,16 @@ def from_cost_matrix(
         )
 
         FacilityModelBuilder.add_facility_constraint(
-            p_dispersion, p_dispersion.problem, p_dispersion.p_facilities
+            p_dispersion, p_dispersion.p_facilities
         )
 
         if predefined_facilities_arr is not None:
             FacilityModelBuilder.add_predefined_facility_constraint(
-                p_dispersion, p_dispersion.problem, predefined_facilities_arr
+                p_dispersion, predefined_facilities_arr
             )
 
         FacilityModelBuilder.add_p_dispersion_interfacility_constraint(
-            p_dispersion, p_dispersion.problem, cost_matrix, r_fac
+            p_dispersion, cost_matrix, r_fac
         )
 
         return p_dispersion
@@ -208,14 +208,14 @@ def from_geodataframe(
             Facility locations.
         facility_col : str
             Facility candidate sites geometry column name.
-        p_facilities: int
+        p_facilities : int
            The number of facilities to be located.
         predefined_facility_col : str (default None)
             Column name representing facilities are already defined.
         distance_metric : str (default 'euclidean')
             A metric used for the distance calculations supported by
             `scipy.spatial.distance.cdist <https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.distance.cdist.html>`_.
-        name: str (default 'P-Dispersion')
+        name : str (default 'P-Dispersion')
             The name of the problem.
 
         Returns

spopt/locate/p_median.py

@@ -99,9 +99,9 @@ def __add_obj(self, range_clients: range, range_facility: range) -> None:
         Parameters
         ----------
 
-        range_clients: range
+        range_clients : range
             The range of demand points.
-        range_facility: range
+        range_facility : range
             The range of facility point.
 
         Returns
@@ -115,7 +115,7 @@ def __add_obj(self, range_clients: range, range_facility: range) -> None:
         self.problem += (
             pulp.lpSum(
                 [
-                    self.aij[i][j] * cli_assgn_vars[i][j]
+                    self.aij[i, j] * cli_assgn_vars[i, j]
                     for i in range_clients
                     for j in range_facility
                 ]
@@ -138,7 +138,7 @@ def from_cost_matrix(
         Parameters
         ----------
 
-        cost_matrix: numpy.array
+        cost_matrix : numpy.array
             A cost matrix in the form of a 2D array between origins and destinations.
         weights : numpy.array
             A 1D array of service load or population demand.
@@ -229,13 +229,14 @@ def from_cost_matrix(
         3.027
 
         """
-        r_cli = range(cost_matrix.shape[0])
+        n_cli = cost_matrix.shape[0]
+        r_cli = range(n_cli)
         r_fac = range(cost_matrix.shape[1])
 
         model = pulp.LpProblem(name, pulp.LpMinimize)
 
         weights_sum = weights.sum()
-        weights = np.reshape(weights, (cost_matrix.shape[0], 1))
+        weights = np.reshape(weights, (n_cli, 1))
         aij = weights * cost_matrix
 
         p_median = PMedian(name, model, aij, weights_sum)
@@ -247,20 +248,14 @@ def from_cost_matrix(
 
         if predefined_facilities_arr is not None:
             FacilityModelBuilder.add_predefined_facility_constraint(
-                p_median, p_median.problem, predefined_facilities_arr
+                p_median, predefined_facilities_arr
             )
 
         p_median.__add_obj(r_cli, r_fac)
 
-        FacilityModelBuilder.add_facility_constraint(
-            p_median, p_median.problem, p_facilities
-        )
-        FacilityModelBuilder.add_assignment_constraint(
-            p_median, p_median.problem, r_fac, r_cli
-        )
-        FacilityModelBuilder.add_opening_constraint(
-            p_median, p_median.problem, r_fac, r_cli
-        )
+        FacilityModelBuilder.add_facility_constraint(p_median, p_facilities)
+        FacilityModelBuilder.add_assignment_constraint(p_median, r_fac, r_cli)
+        FacilityModelBuilder.add_opening_constraint(p_median, r_fac, r_cli)
 
         return p_median
 
@@ -443,7 +438,7 @@ def facility_client_array(self) -> None:
             array_cli = []
             if fac_vars[j].value() > 0:
                 for i in range(len(cli_vars)):
-                    if cli_vars[i][j].value() > 0:
+                    if cli_vars[i, j].value() > 0:
                         array_cli.append(i)
 
             self.fac2cli.append(array_cli)

spopt/tests/test_locate.py

@@ -869,58 +869,56 @@ def setup_method(self) -> None:
     def test_attribute_error_add_set_covering_constraint(self):
         with pytest.raises(AttributeError, match="Before setting coverage constraints"):
             dummy_class = LSCP("dummy", pulp.LpProblem("name"))
-            dummy_matrix = numpy.array([])
+            dummy_class.aij = numpy.array([])
             dummy_range = range(1)
             FacilityModelBuilder.add_set_covering_constraint(
-                dummy_class, dummy_class.problem, dummy_matrix, dummy_range, dummy_range
+                dummy_class, dummy_range, dummy_range
             )
 
     def test_attribute_error_add_facility_constraint(self):
         with pytest.raises(AttributeError, match="Before setting facility constraint"):
-            dummy_class = LSCP("dummy", pulp.LpProblem("name"))
+            dummy_class = MCLP("dummy", pulp.LpProblem("name"))
             dummy_p_facility = 1
-            FacilityModelBuilder.add_facility_constraint(
-                dummy_class, dummy_class.problem, 1
-            )
+            FacilityModelBuilder.add_facility_constraint(dummy_class, 1)
 
     def test_attribute_error_add_maximal_coverage_constraint(self):
         with pytest.raises(
             AttributeError, match="Before setting maximal coverage constraints"
         ):
-            dummy_class = LSCP("dummy", pulp.LpProblem("name"))
-            dummy_matrix = numpy.array([])
+            dummy_class = MCLP("dummy", pulp.LpProblem("name"))
+            dummy_class.aij = numpy.array([])
             dummy_range = range(1)
             FacilityModelBuilder.add_maximal_coverage_constraint(
-                dummy_class, dummy_class.problem, dummy_matrix, dummy_range, dummy_range
+                dummy_class, dummy_range, dummy_range
             )
 
     def test_attribute_error_add_assignment_constraint(self):
         with pytest.raises(
             AttributeError, match="Before setting assignment constraints"
         ):
-            dummy_class = LSCP("dummy", pulp.LpProblem("name"))
+            dummy_class = PMedian("dummy", pulp.LpProblem("name"), numpy.array([]), 1)
             dummy_range = range(1)
             FacilityModelBuilder.add_assignment_constraint(
-                dummy_class, dummy_class.problem, dummy_range, dummy_range
+                dummy_class, dummy_range, dummy_range
             )
 
     def test_attribute_error_add_opening_constraint(self):
         with pytest.raises(AttributeError, match="Before setting opening constraints"):
-            dummy_class = LSCP("dummy", pulp.LpProblem("name"))
+            dummy_class = PMedian("dummy", pulp.LpProblem("name"), numpy.array([]), 1)
             dummy_range = range(1)
             FacilityModelBuilder.add_opening_constraint(
-                dummy_class, dummy_class.problem, dummy_range, dummy_range
+                dummy_class, dummy_range, dummy_range
             )
 
     def test_attribute_error_add_minimized_maximum_constraint(self):
         with pytest.raises(
             AttributeError, match="Before setting minimized maximum constraints"
         ):
-            dummy_class = LSCP("dummy", pulp.LpProblem("name"))
             dummy_matrix = numpy.array([])
+            dummy_class = PCenter("dummy", pulp.LpProblem("name"), dummy_matrix)
             dummy_range = range(1)
             FacilityModelBuilder.add_minimized_maximum_constraint(
-                dummy_class, dummy_class.problem, dummy_matrix, dummy_range, dummy_range
+                dummy_class, dummy_matrix, dummy_range, dummy_range
             )
 
     def test_error_lscp_different_crs(self):