Add benchmark tests and post PR comments c.t. base

- lints

.github/workflows/push.yml

@@ -34,9 +34,9 @@ jobs:
         pip install -e .[test]
     - name: Run Ruff
       run: ruff check --output-format=github .
-    - name: Run tests with coverage
+    - name: Run tests with coverage and benchmarking
       run: |
-          pytest --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=network_wrangler tests/ | tee pytest-coverage.txt
+          pytest --junitxml=pytest.xml --cov-report=term-missing:skip-covered --cov=network_wrangler tests/ | tee pytest-coverage.txt --benchmark-save=pr_benchmark --benchmark-json=pr_benchmark.json
     - name: Pytest coverage comment
       if: github.event_name == 'pull_request'
       uses: MishaKav/pytest-coverage-comment@main
@@ -46,3 +46,35 @@ jobs:
     - name: If a pull request, add a comment about coverage
       if: github.event_name == 'pull_request'
       uses: coroo/pytest-coverage-commentator@v1.0.2
+    - name: Retrieve benchmark results from base branch
+      if: github.event_name == 'pull_request'
+      id: compare
+      run: |
+        git checkout ${{ github.event.pull_request.base.ref }}
+        if [ -f "tests/.benchmarks/benchmark_results.json" ]; then
+          cp tests/.benchmarks/benchmark_results.json base_benchmark.json
+          echo "::set-output name=benchmark_exists::true"
+        else
+          echo "::set-output name=benchmark_exists::false"
+        fi
+    - name: Compare benchmarks
+      if: steps.compare.outputs.benchmark_exists == 'true'
+      run: |
+        pytest-benchmark compare base_benchmark.json pr_benchmark.json --csv=comparison.csv --json=comparison.json --histogram=histogram.png
+
+    - name: Post comparison comment on pull request
+      if: steps.compare.outputs.benchmark_exists == 'true'
+      uses: marocchino/sticky-pull-request-comment@v2
+      with:
+        header: 'Benchmark Comparison'
+        message: |
+          ## Benchmark Results
+          ![Benchmark Comparison](./histogram.png)
+
+          | Test name | PR Benchmark | Base Benchmark | Difference |
+          | --------- | ------------ | -------------- | ---------- |
+          ```bash
+          pytest-benchmark compare base_benchmark.json pr_benchmark.json --csv --diff
+          ```
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

network_wrangler/roadway/links/create.py

@@ -38,7 +38,6 @@ class LinkAddError(Exception):
     pass
 
 
-
 class LinkCreationError(Exception):
     """Raised when there is an issue with creating links."""
 

network_wrangler/roadway/model_roadway.py

@@ -176,7 +176,7 @@ def write(
             prefix=prefix,
             file_format=file_format,
             overwrite=overwrite,
-            true_shape=true_shape
+            true_shape=true_shape,
         )
 
 

network_wrangler/roadway/network.py

@@ -424,9 +424,7 @@ def add_shapes(self, add_shapes_df: Union[pd.DataFrame, DataFrame[RoadShapesTabl
         """
         dupe_ids = self.shapes_df.shape_id.isin(add_shapes_df.shape_id)
         if dupe_ids.any():
-            WranglerLogger.error(
-                f"Cannot add shapes with shape_id already in network: {dupe_ids}"
-            )
+            WranglerLogger.error(f"Cannot add shapes with shape_id already in network: {dupe_ids}")
             raise ShapeAddError("Cannot add shapes with shape_id already in network.")
         if not isinstance(add_shapes_df, RoadShapesTable):
             add_shapes_df = df_to_shapes_df(add_shapes_df)

network_wrangler/roadway/nodes/edit.py

@@ -23,6 +23,7 @@
 
 class NodeChangeError(Exception):
     """Raised when there is an issue with applying a node change."""
+
     pass
 
 

network_wrangler/roadway/shapes/create.py

@@ -22,6 +22,7 @@
 
 class ShapeAddError(Exception):
     """Raised when there is an issue with adding shapes."""
+
     pass
 
 

network_wrangler/utils/models.py

@@ -212,6 +212,7 @@ def check_type_hint(value):
 
 def validate_call_pyd(func):
     """Decorator to validate the function i/o using Pydantic models without Pandera."""
+
     @wraps(func)
     def wrapper(*args, **kwargs):
         type_hints = get_type_hints(func)

pytest.ini

@@ -1,7 +1,6 @@
 [pytest]
-addopts = --profile
+addopts = --profile --benchmark-min-rounds=1 --benchmark-save=benchmark_results --benchmark-save-data
 markers =
-    profile: mark a test as a profile test
     skipci: mark a test as a test that should be skipped in CI
     failing: tests that are known to be failing and need to be fixed
 log_cli = true

tests/test_benchmarks.py

@@ -1,74 +1,98 @@
-"""Benchmark tests for the project."""
-
 from network_wrangler.roadway import load_roadway_from_dir, write_roadway
 from network_wrangler.transit import load_transit, write_transit
 from projectcard import read_cards
 
 
-def test_roadway_io(stpaul_ex_dir, test_out_dir):
+def roadway_io(stpaul_ex_dir, test_out_dir):
     net = load_roadway_from_dir(stpaul_ex_dir)
-    write_roadway(net, out_dir=test_out_dir, prefix="stpaul", file_format="geojson")
+    write_roadway(net, test_out_dir, prefix="stpaul", file_format="geojson")
+
 
+def test_roadway_io(benchmark, stpaul_ex_dir, test_out_dir):
+    benchmark(roadway_io, stpaul_ex_dir, test_out_dir)
 
-def test_roadway_property_change(stpaul_ex_dir):
+
+def roadway_property_change(stpaul_ex_dir):
     net = load_roadway_from_dir(stpaul_ex_dir)
     c = stpaul_ex_dir / "project_cards" / "road.prop_change.multiple.yml"
     p = read_cards(c)
     for p in p.values():
         net.apply(p)
 
 
-def test_roadway_managed_lane_model_net(stpaul_ex_dir, test_out_dir):
+def test_roadway_property_change(benchmark, stpaul_ex_dir):
+    benchmark(roadway_property_change, stpaul_ex_dir)
+
+
+def roadway_model_net(stpaul_ex_dir, test_out_dir):
     net = load_roadway_from_dir(stpaul_ex_dir)
     c = stpaul_ex_dir / "project_cards" / "road.managed_lanes.restricted_access.yml"
-    p = read_cards(c)
-    for p in p.values():
-        net.apply(p)
+    card_dict = read_cards(c)
+    for p in card_dict.values():
+        net = net.apply(p)
     net.model_net.write(test_out_dir / "stpaul_managed_lane_model_net")
 
 
-def test_roadway_many_projects_io(stpaul_ex_dir):
+def test_roadway_managed_lane_model_net(benchmark, stpaul_ex_dir, test_out_dir):
+    benchmark(roadway_model_net, stpaul_ex_dir, test_out_dir)
+
+
+def apply_many_road_projects(stpaul_ex_dir):
     net = load_roadway_from_dir(stpaul_ex_dir)
-    projects = [
-        "road.divide_facility.yml",
-        "road.prop_change.multiple.yml"
-    ]
+    projects = ["road.divide_facility.yml", "road.prop_change.multiple.yml"]
     c = [stpaul_ex_dir / "project_cards" / p for p in projects]
-    p = read_cards(c)
-    for p in p.values():
+    card_dict = read_cards(c)
+    for p in card_dict.values():
         net.apply(p)
 
 
-def test_transit_io(stpaul_ex_dir, test_out_dir):
+def test_roadway_many_projects_io(benchmark, stpaul_ex_dir):
+    benchmark(apply_many_road_projects, stpaul_ex_dir)
+
+
+def transit_io(stpaul_ex_dir, test_out_dir):
     net = load_transit(stpaul_ex_dir)
     write_transit(net, test_out_dir)
 
 
-def test_transit_property_change(stpaul_ex_dir):
+def test_transit_io(benchmark, stpaul_ex_dir, test_out_dir):
+    benchmark(transit_io, stpaul_ex_dir, test_out_dir)
+
+
+def apply_transit_property_change(stpaul_ex_dir):
     net = load_transit(stpaul_ex_dir)
     c = stpaul_ex_dir / "project_cards" / "transit.prop_change.route_time.yml"
-    p = read_cards(c)
-    for p in p.values():
+    card_dict = read_cards(c)
+    for p in card_dict.values():
         net.apply(p)
 
 
-def test_multiple_transit_property_change(stpaul_ex_dir):
+def test_transit_property_change(benchmark, stpaul_ex_dir):
+    benchmark(apply_transit_property_change, stpaul_ex_dir)
+
+
+def apply_multiple_transit_projects(stpaul_ex_dir):
     net = load_transit(stpaul_ex_dir)
-    projects = [
-        "transit.prop_change.route_time.yml",
-        "transit.prop_change.multiple_trip_time.yml"
-    ]
+    projects = ["transit.prop_change.route_time.yml", "transit.prop_change.multiple_trip_time.yml"]
     c = [stpaul_ex_dir / "project_cards" / p for p in projects]
-    p = read_cards(c)
-    for p in p.values():
+    cards_dict = read_cards(c)
+    for p in cards_dict.values():
         net.apply(p)
 
 
-def test_transit_routing_change(stpaul_ex_dir):
+def test_multiple_transit_property_change(benchmark, stpaul_ex_dir):
+    benchmark(apply_multiple_transit_projects, stpaul_ex_dir)
+
+
+def apply_transit_routing_change(stpaul_ex_dir):
     road_net = load_roadway_from_dir(stpaul_ex_dir)
     net = load_transit(stpaul_ex_dir)
     net.road_net = road_net
     c = stpaul_ex_dir / "project_cards" / "transit.routing_change.yml"
     p = read_cards(c)
     for p in p.values():
         net.apply(p)
+
+
+def test_transit_routing_change(benchmark, stpaul_ex_dir):
+    benchmark(apply_transit_routing_change, stpaul_ex_dir)

tests/test_roadway/test_changes/test_managed_lanes.py

@@ -42,7 +42,7 @@
     },
 }
 
-@pytest.mark.profile
+
 def test_add_managed_lane(request, stpaul_net, stpaul_ex_dir, scratch_dir):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 
@@ -129,7 +129,7 @@ def test_add_managed_lane(request, stpaul_net, stpaul_ex_dir, scratch_dir):
 
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
-@pytest.mark.profile
+
 def test_managed_lane_change_functionality(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Starting: {request.node.name}")
     net = copy.deepcopy(stpaul_net)
@@ -168,7 +168,7 @@ def test_managed_lane_change_functionality(request, stpaul_net, stpaul_ex_dir):
 
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
-@pytest.mark.profile
+
 def test_existing_managed_lane_apply(request, stpaul_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 

tests/test_roadway/test_changes/test_roadway_add_delete.py

@@ -12,7 +12,7 @@
 Usage:  `pytest tests/test_roadway/test_changes/test_roadway_add_delete.py`
 """
 
-@pytest.mark.profile
+
 def test_add_roadway_link_project_card(request, small_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 
@@ -69,7 +69,6 @@ def test_add_roadway_link_project_card(request, small_net):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_add_roadway_project_card(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 
@@ -92,7 +91,6 @@ def test_add_roadway_project_card(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_add_delete_roadway_project_card(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 
@@ -115,7 +113,6 @@ def test_add_delete_roadway_project_card(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-
 def test_delete_roadway_shape(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 
@@ -137,7 +134,6 @@ def test_delete_roadway_shape(request, stpaul_net, stpaul_ex_dir):
     print("--Finished:", request.node.name)
 
 
-@pytest.mark.profile
 def test_add_nodes(request, small_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
     net = copy.deepcopy(small_net)

tests/test_roadway/test_changes/test_roadway_feature_change.py

@@ -8,7 +8,6 @@
 from projectcard import read_card
 
 
-@pytest.mark.profile
 def test_change_roadway_existing_and_change_single_link(request, stpaul_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
     net = copy.deepcopy(stpaul_net)
@@ -59,7 +58,6 @@ def test_change_roadway_existing_and_change_single_link(request, stpaul_net):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_change_multiple_properties_multiple_links(request, stpaul_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
     net = copy.deepcopy(stpaul_net)
@@ -116,7 +114,6 @@ def test_change_multiple_properties_multiple_links(request, stpaul_net):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_change_multiple_properties_multiple_links_existing_set(request, stpaul_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
     net = copy.deepcopy(stpaul_net)
@@ -250,7 +247,6 @@ def test_add_adhoc_field_from_card(request, stpaul_net, stpaul_ex_dir):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_change_node_xy(request, small_net):
     """Tests if X and Y property changes from a project card also update the node/link geometry."""
     WranglerLogger.info(f"--Starting: {request.node.name}")

tests/test_roadway/test_io.py

@@ -126,7 +126,7 @@ def test_roadway_model_coerce(small_net):
     WranglerLogger.debug(f"small_net.links_df.cols: \n{small_net.links_df.columns}")
     assert "osm_link_id" in small_net.links_df.columns
 
-@pytest.mark.profile
+
 @pytest.mark.parametrize("io_format", ["geojson", "parquet"])
 @pytest.mark.parametrize("ex", ["stpaul", "small"])
 def test_roadway_geojson_read_write_read(example_dir, test_out_dir, ex, io_format):
@@ -156,7 +156,7 @@ def test_load_roadway_no_shapes(tmpdir, example_dir):
     assert not roadway_network.nodes_df.empty
     assert roadway_network._shapes_df is None
 
-@pytest.mark.profile
+
 def test_load_roadway_within_boundary(tmpdir, example_dir):
     one_block = Polygon(
         [

tests/test_roadway/test_model_roadway.py

@@ -60,7 +60,6 @@ def test_add_adhoc_managed_lane_field(request, small_net):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-
 def test_create_ml_network_shape(request, small_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 
@@ -149,7 +148,6 @@ def test_create_ml_network_shape(request, small_net):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_managed_lane_restricted_access_egress(request, stpaul_net, stpaul_ex_dir):
     """Tests usage of ML_access/egress_point when they are set to a list of nodes instead of "all".
 

tests/test_scenario.py

@@ -191,7 +191,7 @@ def test_apply_summary_wrappers(request, stpaul_card_dir, stpaul_net, stpaul_tra
 
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
-@pytest.mark.profile
+
 def test_scenario_building_from_script(request):
     WranglerLogger.info(f"--Starting: {request.node.name}")
 

tests/test_transit/test_changes/test_transit_prop_changes.py

@@ -95,7 +95,6 @@ def test_coerce_over24hr_times(request, small_transit_net):
     WranglerLogger.info(f"--Finished: {request.node.name}")
 
 
-@pytest.mark.profile
 def test_transit_property_change(request, small_transit_net):
     WranglerLogger.info(f"--Starting: {request.node.name}")
     net = copy.deepcopy(small_transit_net)