Merge pull request #449 from jGaboardi/geometry_crs_warning

testing code cleanup

.github/workflows/testing.yml

@@ -76,26 +76,12 @@
            pytest spopt \
            -v \
            -r a \
-           -n auto \
+           -n logical \
            --color yes \
            --cov spopt \
            --cov-append \
            --cov-report term-missing \
-           --cov-report xml
+           --cov-report xml .
        
        - name: codecov
-         uses: codecov/codecov-action@v3
-         with:
-           token: ${{ secrets.CODECOV_TOKEN }}
-           file: ./coverage.xml
-           name: spopt-codecov
-
-       - name: Generate and publish the report
-         if: |
-           failure()
-           && steps.status.outcome == 'failure'
-           && github.event_name == 'schedule'
-           && github.repository_owner == 'pysal'
-         uses: xarray-contrib/issue-from-pytest-log@v1
-         with:
-           log-path: pytest-log.jsonl
+         uses: codecov/codecov-action@v4

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 files: "spopt\/"
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: "v0.3.5"
+    rev: "v0.4.2"
     hooks:
       - id: ruff
       - id: ruff-format

spopt/locate/util.py

@@ -1,6 +1,6 @@
 import geopandas
 import numpy
-from shapely.geometry import MultiPolygon, Point, Polygon
+from shapely import MultiPolygon, Point, Polygon
 
 
 def simulated_geo_points(
@@ -17,7 +17,7 @@ def simulated_geo_points(
     Parameters
     ----------
 
-    in_data : geopandas.GeoDataFrame, shapely.geometry.{Polygon, MultiPolygon}
+    in_data : geopandas.GeoDataFrame, shapely.{Polygon, MultiPolygon}
         The areal unit in which to generate points.
     needed : int (default 1)
         The number of points to generate.

spopt/tests/conftest.py

@@ -1,7 +1,11 @@
+import pathlib
+import pickle
 import warnings
 
+import _pytest
 import geopandas
 import numpy
+import pandas
 import pytest
 import shapely
 
@@ -19,6 +23,29 @@
 GPD_GE_10 = Version(geopandas.__version__) >= Version("1.0")
 
 
+def dirpath() -> pathlib.Path:
+    """Path to test data directory"""
+    return pathlib.Path(__file__).absolute().parent / "data"
+
+
+@pytest.fixture
+def load_test_data():
+    """Load test data for the ``locate`` module."""
+
+    def _load_test_data(_file: str) -> dict | pandas.DataFrame:
+        if _file.endswith(".pkl"):
+            with open(dirpath() / _file, "rb") as f:
+                test_data = pickle.load(f)
+        elif _file.endswith(".csv"):
+            test_data = pandas.read_csv(dirpath() / _file)
+        else:
+            raise FileNotFoundError(f"`{_file}` does not exist.")
+
+        return test_data
+
+    return _load_test_data
+
+
 @pytest.fixture
 def network_instance():
     """Return:
@@ -101,3 +128,77 @@ def _network_instance(
         return clients_snapped, facilities_snapped, cost_matrix
 
     return _network_instance
+
+
+_warns_geo_crs = pytest.warns(UserWarning, match="Geometry is in a geographic CRS")
+
+
+@pytest.fixture
+def loc_warns_geo_crs() -> _pytest.recwarn.WarningsChecker:
+    """`locate` warning"""
+    return _warns_geo_crs
+
+
+@pytest.fixture
+def loc_warns_mixed_type_dem() -> _pytest.recwarn.WarningsChecker:
+    """`locate` warning"""
+    return pytest.warns(UserWarning, match="Demand geodataframe contains mixed type")
+
+
+@pytest.fixture
+def loc_warns_mixed_type_fac() -> _pytest.recwarn.WarningsChecker:
+    """`locate` warning"""
+    return pytest.warns(UserWarning, match="Facility geodataframe contains mixed type")
+
+
+@pytest.fixture
+def loc_raises_diff_crs() -> _pytest.python_api.RaisesContext:
+    """`locate` error"""
+    return pytest.raises(ValueError, match="Geodataframes crs are different: ")
+
+
+@pytest.fixture
+def loc_raises_infeasible() -> _pytest.python_api.RaisesContext:
+    """`locate` error"""
+    return pytest.raises(RuntimeError, match="Model is not solved: Infeasible.")
+
+
+@pytest.fixture
+def loc_raises_fac_constr() -> _pytest.python_api.RaisesContext:
+    """`locate` error"""
+    return pytest.raises(AttributeError, match="Before setting facility constraint")
+
+
+@pytest.fixture
+def toy_fac_data() -> geopandas.GeoDataFrame:
+    """Toy facility data used in ``locate`` error & warning tests."""
+    pol1 = shapely.Polygon([(0, 0), (1, 0), (1, 1)])
+    pol2 = shapely.Polygon([(0, 0), (1, 0), (1, 1), (0, 1)])
+    pol3 = shapely.Polygon([(2, 0), (3, 0), (3, 1), (2, 1)])
+    polygon_dict = {"geometry": [pol1, pol2, pol3]}
+
+    return geopandas.GeoDataFrame(polygon_dict, crs="EPSG:4326")
+
+
+@pytest.fixture
+def toy_dem_data() -> (
+    tuple[geopandas.GeoDataFrame, geopandas.GeoDataFrame, geopandas.GeoDataFrame]
+):
+    """Toy demand data used in ``locate`` error & warning tests."""
+
+    point = shapely.Point(10, 10)
+    point_dict = {"weight": 4, "geometry": [point]}
+
+    gdf_dem = geopandas.GeoDataFrame(point_dict, crs="EPSG:4326")
+
+    with warnings.catch_warnings():
+        warnings.filterwarnings(
+            "ignore", message="Conversion of an array with ndim > 0"
+        )
+        gdf_dem_crs = gdf_dem.to_crs("EPSG:3857")
+
+    gdf_dem_buffered = gdf_dem.copy()
+    with _warns_geo_crs:
+        gdf_dem_buffered["geometry"] = gdf_dem.buffer(2)
+
+    return gdf_dem, gdf_dem_crs, gdf_dem_buffered

spopt/tests/test_c_p_median.py

@@ -1,21 +1,14 @@
-import os
-
 import numpy
-import pandas
 import pulp
 import pytest
 
 from spopt.locate import PMedian
-from spopt.locate.base import (
-    SpecificationError,
-)
+from spopt.locate.base import SpecificationError
 
 
 class TestSyntheticLocate:
     @pytest.fixture(autouse=True)
     def setup_method(self, network_instance) -> None:
-        self.dirpath = os.path.join(os.path.dirname(__file__), "./data/")
-
         client_count, facility_count = 2, 3
         (
             self.clients_snapped,
@@ -66,7 +59,9 @@ def test_c_p_median_with_predefined_facilities_from_cost_matrix(self):
         observed = result.fac2cli
         assert known == observed
 
-    def test_c_p_median_with_predefined_facilities_infeasible(self):
+    def test_c_p_median_with_predefined_facilities_infeasible(
+        self, loc_raises_infeasible
+    ):
         facility_capacity = numpy.array([5, 7, 10])
         demand_quantity = numpy.array([4, 10])
         predefine = numpy.array([0])
@@ -78,17 +73,15 @@ def test_c_p_median_with_predefined_facilities_infeasible(self):
             predefined_facilities_arr=predefine,
             fulfill_predefined_fac=True,
         )
-        with pytest.raises(RuntimeError, match="Model is not solved: Infeasible."):
+        with loc_raises_infeasible:
             p_median.solve(pulp.PULP_CBC_CMD(msg=False))
 
 
 class TestRealWorldLocate:
-    def setup_method(self) -> None:
-        self.dirpath = os.path.join(os.path.dirname(__file__), "./data/")
+    @pytest.fixture(autouse=True)
+    def setup_method(self, load_test_data) -> None:
+        time_table = load_test_data("example_subject_student_school_journeys.csv")
 
-        time_table = pandas.read_csv(
-            self.dirpath + "example_subject_student_school_journeys.csv"
-        )
         self.cost_matrix = (
             time_table.pivot_table(
                 columns="school",
@@ -101,12 +94,8 @@ def setup_method(self) -> None:
             .values
         )
 
-        self.demand_points = pandas.read_csv(
-            self.dirpath + "example_subject_students.csv"
-        )
-        self.facility_points = pandas.read_csv(
-            self.dirpath + "example_subject_schools.csv"
-        )
+        self.demand_points = load_test_data("example_subject_students.csv")
+        self.facility_points = load_test_data("example_subject_schools.csv")
 
         self.p_facility = 10
         self.demand = numpy.ones(len(self.demand_points))
@@ -128,11 +117,11 @@ def test_optimality_capacitated_pmedian_with_predefined_facilities(self):
         pmedian = pmedian.solve(pulp.PULP_CBC_CMD(msg=False))
         assert pmedian.problem.status == pulp.LpStatusOptimal
 
-    def test_infeasibility_capacitated_pmedian(self):
+    def test_infeasibility_capacitated_pmedian(self, loc_raises_infeasible):
         pmedian = PMedian.from_cost_matrix(
             self.cost_matrix, self.demand, 0, facility_capacities=self.capacities_arr
         )
-        with pytest.raises(RuntimeError, match="Model is not solved: Infeasible."):
+        with loc_raises_infeasible:
             pmedian.solve(pulp.PULP_CBC_CMD(msg=False))
 
     def test_mixin_mean_time(self):

spopt/tests/test_clscp-so.py

@@ -1,5 +1,4 @@
 # ruff: noqa: N999
-
 import numpy
 import pulp
 import pytest
@@ -94,7 +93,7 @@ def test_clscpso_dem_gt_cap_error(self):
                 demand_quantity_arr=demand_quantity,
             )
 
-    def test_clscpso_infease_error(self):
+    def test_clscpso_infease_error(self, loc_raises_infeasible):
         service_radius = 1
         facility_capacity = numpy.array([5, 15])
         demand_quantity = numpy.arange(1, 6)
@@ -104,5 +103,5 @@ def test_clscpso_infease_error(self):
             facility_capacity_arr=facility_capacity,
             demand_quantity_arr=demand_quantity,
         )
-        with pytest.raises(RuntimeError, match="Model is not solved:"):
+        with loc_raises_infeasible:
             clscpso.solve(pulp.PULP_CBC_CMD(msg=False))

spopt/tests/test_knearest_p_median.py

@@ -2,11 +2,13 @@
 import numpy
 import pulp
 import pytest
-from shapely.geometry import Point
+from shapely import Point
 
 from spopt.locate.base import SpecificationError
 from spopt.locate.p_median import KNearestPMedian
 
+loc_raises_val_k = pytest.raises(ValueError, match="The value of `k` should be")
+
 
 class TestKNearestPMedian:
     def setup_method(self) -> None:
@@ -64,7 +66,7 @@ def test_solve(self):
 
     def test_error_overflow_k(self):
         k = numpy.array([10, 10])
-        with pytest.raises(ValueError, match="The value of `k` should be"):
+        with loc_raises_val_k:
             KNearestPMedian.from_geodataframe(
                 self.gdf_demand,
                 self.gdf_fac,
@@ -92,7 +94,7 @@ def test_error_k_array_non_numpy_array(self):
 
     def test_error_k_array_invalid_value(self):
         k = numpy.array([1, 4])
-        with pytest.raises(ValueError, match="The value of `k` should be no more "):
+        with loc_raises_val_k:
             KNearestPMedian.from_geodataframe(
                 self.gdf_demand,
                 self.gdf_fac,
@@ -134,10 +136,10 @@ def test_error_no_crs_facility(self):
                 k_array=k,
             )
 
-    def test_error_geodataframe_crs_mismatch(self):
+    def test_error_geodataframe_crs_mismatch(self, loc_raises_diff_crs):
         _gdf_fac = self.gdf_fac.copy().to_crs("EPSG:3857")
         k = numpy.array([1, 1])
-        with pytest.raises(ValueError, match="Geodataframes crs are different"):
+        with loc_raises_diff_crs:
             KNearestPMedian.from_geodataframe(
                 self.gdf_demand,
                 _gdf_fac,