From c61b021978c3caaa6d3c8d879574d5658fbeb3ba Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 6 Feb 2024 09:59:11 +0000
Subject: [PATCH 01/17] Oneline json.loads in load_graph

---
 swmmanywhere/graph_utilities.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 0b4acabb..e2967e40 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -33,9 +33,7 @@ def load_graph(fid: Path) -> nx.Graph:
     Returns:
         G (nx.Graph): A graph
     """
-    # Define the custom decoding function    
-    with open(fid, 'r') as file:
-        json_data = json.load(file)
+    json_data = json.loads(fid.read_text())
 
     G = nx.node_link_graph(json_data,directed=True)
     for u, v, data in G.edges(data=True):

From abea9f3c11f08959726e4b110ac257388f536c7b Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 6 Feb 2024 10:08:05 +0000
Subject: [PATCH 02/17] Use f-string formatting

---
 swmmanywhere/graph_utilities.py | 13 +++++--------
 swmmanywhere/prepare_data.py    |  4 ++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index e2967e40..86bb26a5 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -95,12 +95,10 @@ def validate_requirements(self,
                               node_attributes: set) -> None:
         """Validate that the graph has the required attributes."""
         for attribute in self.required_edge_attributes:
-            assert attribute in edge_attributes, "{0} not in attributes".format(
-                attribute)
+            assert attribute in edge_attributes, f"{attribute} not in attributes"
             
         for attribute in self.required_node_attributes:
-            assert attribute in node_attributes, "{0} not in attributes".format(
-                attribute)
+            assert attribute in node_attributes, f"{attribute} not in attributes"
             
 
     def add_graphfcn(self,
@@ -235,7 +233,7 @@ def __call__(self, G: nx.Graph, **kwargs) -> nx.Graph:
         for u, v, data in G.edges(data=True):
             if (v, u) not in G.edges:
                 reverse_data = data.copy()
-                reverse_data['id'] = '{0}.reversed'.format(data['id'])
+                reverse_data['id'] = f"{data['id']}.reversed"
                 G_new.add_edge(v, u, **reverse_data)
         return G_new
 
@@ -304,12 +302,11 @@ def create_new_edge_data(line, data, id_):
                             new_data = create_new_edge_data([start, 
                                                             end], 
                                                             data, 
-                                                            '{0}.{1}'.format(
-                                                                data['id'],ix))
+                                                            f"{data['id']}.{ix}")
                             if (v,u) in graph.edges:
                                 # Create reversed data
                                 data_r = graph.get_edge_data(v, u).copy()[0]
-                                id_ = '{0}.{1}'.format(data_r['id'],ix)
+                                id_ = f"{data_r['id']}.{ix}"
                                 new_data_r = create_new_edge_data([end, start], 
                                                                 data_r.copy(), 
                                                                 id_)
diff --git a/swmmanywhere/prepare_data.py b/swmmanywhere/prepare_data.py
index 951e5f38..bdbbdafb 100644
--- a/swmmanywhere/prepare_data.py
+++ b/swmmanywhere/prepare_data.py
@@ -52,7 +52,7 @@ def get_country(x: float,
         data = yaml.safe_load(file)
 
     # Get country ISO code from coordinates
-    location = geolocator.reverse("{0}, {1}".format(y, x), exactly_one=True)
+    location = geolocator.reverse(f"{y}, {x}", exactly_one=True)
     iso_country_code = location.raw.get("address", {}).get("country_code")
     iso_country_code = iso_country_code.upper()
 
@@ -219,7 +219,7 @@ def download_precipitation(bbox: tuple[float, float, float, float],
         }
 
     c = cdsapi.Client(url='https://cds.climate.copernicus.eu/api/v2', 
-                      key='{0}:{1}'.format(username, api_key))
+                      key=f'{username}:{api_key}')
     # Get data
     c.retrieve('reanalysis-era5-single-levels', 
                request, 

From 1658ebc2a9d4eb9f7e9232174cd00a8abe13c0f6 Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 6 Feb 2024 10:33:00 +0000
Subject: [PATCH 03/17] Tidy up contributing area assignment

---
 swmmanywhere/graph_utilities.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 86bb26a5..068d2ce3 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -425,15 +425,17 @@ def __call__(self, G: nx.Graph,
 
         # Assign contributing area
         imperv_lookup = subs_rc.set_index('id').impervious_area.to_dict()
+        
+        # Set node attributes
+        nx.set_node_attributes(G, 0.0, 'contributing_area')
         nx.set_node_attributes(G, imperv_lookup, 'contributing_area')
-        for u, d in G.nodes(data=True):
-            if 'contributing_area' not in d.keys():
-                d['contributing_area'] = 0.0
-        for u,v,d in G.edges(data=True):
-            if u in imperv_lookup.keys():
-                d['contributing_area'] = imperv_lookup[u]
-            else:
-                d['contributing_area'] = 0.0
+
+        # Prepare edge attributes
+        edge_attributes = {edge: G.nodes[edge[0]]['contributing_area'] 
+                           for edge in G.edges}
+
+        # Set edge attributes
+        nx.set_edge_attributes(G, edge_attributes, 'contributing_area')
         return G
 
 @register_graphfcn

From 85d09a7661b8f3af2b6e0839e4c1eb883f6d9303 Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 6 Feb 2024 15:43:29 +0000
Subject: [PATCH 04/17] Tidy bounds normalising in weights

---
 swmmanywhere/graph_utilities.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 068d2ce3..af0edc6d 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -6,10 +6,11 @@
 import json
 import tempfile
 from abc import ABC, abstractmethod
+from collections import defaultdict
 from heapq import heappop, heappush
 from itertools import product
 from pathlib import Path
-from typing import Callable, Hashable
+from typing import Any, Callable, Dict, Hashable, List
 
 import geopandas as gpd
 import networkx as nx
@@ -576,15 +577,13 @@ def __call__(self, G: nx.Graph,
             G (nx.Graph): A graph
         """
         # Calculate bounds to normalise between
-        bounds = {}
-        for weight in topo_derivation.weights:
-            bounds[weight] = [np.Inf, -np.Inf]
+        bounds: Dict[Any, List[float]] = defaultdict(lambda: [np.Inf, -np.Inf])
+
+        for (u, v, d), w in product(G.edges(data=True), 
+                                    topo_derivation.weights):
+            bounds[w][0] = min(bounds[w][0], d.get(w, np.Inf))
+            bounds[w][1] = max(bounds[w][1], d.get(w, -np.Inf))
 
-        for u, v, d in G.edges(data=True):
-            for attr, bds in bounds.items():
-                bds[0] = min(bds[0], d[attr])
-                bds[1] = max(bds[1], d[attr])
-        
         G = G.copy()
         for u, v, d in G.edges(data=True):
             total_weight = 0

From 83ec08c721299a6e2b61753f5852c243894d6f9c Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 6 Feb 2024 15:45:55 +0000
Subject: [PATCH 05/17] Convert set_slope to comprehension

---
 swmmanywhere/graph_utilities.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index af0edc6d..f1601d23 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -498,9 +498,13 @@ def __call__(self, G: nx.Graph,
             G (nx.Graph): A graph
         """
         G = G.copy()
-        for u,v,d in G.edges(data=True):
-            slope = (G.nodes[u]['elevation'] - G.nodes[v]['elevation']) / d['length']
-            d['surface_slope'] = slope
+        # Compute the slope for each edge
+        slope_dict = {(u, v, k): (G.nodes[u]['elevation'] - G.nodes[v]['elevation']) 
+                        / d['length'] for u, v, k, d in G.edges(data=True,
+                                                                keys=True)}
+
+        # Set the 'surface_slope' attribute for all edges
+        nx.set_edge_attributes(G, slope_dict, 'surface_slope')
         return G
 
 @register_graphfcn

From c35616b417463ac2d1456d500f0a444f03fccfc8 Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 6 Feb 2024 16:27:43 +0000
Subject: [PATCH 06/17] Use shapely for geoms rather than shapely.geom

---
 swmmanywhere/graph_utilities.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index f1601d23..5cfe9977 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -17,8 +17,7 @@
 import numpy as np
 import osmnx as ox
 import pandas as pd
-from shapely import geometry as sgeom
-from shapely import wkt
+import shapely
 from tqdm import tqdm
 
 from swmmanywhere import geospatial_utilities as go
@@ -40,7 +39,7 @@ def load_graph(fid: Path) -> nx.Graph:
     for u, v, data in G.edges(data=True):
         if 'geometry' in data:
             geometry_coords = data['geometry']
-            line_string = sgeom.LineString(wkt.loads(geometry_coords))
+            line_string = shapely.LineString(shapely.wkt.loads(geometry_coords))
             data['geometry'] = line_string
     return G
 
@@ -54,7 +53,7 @@ def save_graph(G: nx.Graph,
     """
     json_data = nx.node_link_data(G)
     def serialize_line_string(obj):
-        if isinstance(obj, sgeom.LineString):
+        if isinstance(obj, shapely.LineString):
             return obj.wkt
         else:
             return obj
@@ -281,11 +280,11 @@ def __call__(self,
         ll = 0
 
         def create_new_edge_data(line, data, id_):
-            new_line = sgeom.LineString(line)
+            new_line = shapely.LineString(line)
             new_data = data.copy()
             new_data['id'] = id_
             new_data['length'] = new_line.length
-            new_data['geometry'] =  sgeom.LineString([(x[0], x[1]) 
+            new_data['geometry'] =  shapely.LineString([(x[0], x[1]) 
                                                     for x in new_line.coords])
             return new_data
 
@@ -294,7 +293,7 @@ def create_new_edge_data(line, data, id_):
             length = data['length']
             if ((u, v) not in edges_to_remove) & ((v, u) not in edges_to_remove):
                 if length > max_length:
-                    new_points = [sgeom.Point(x) 
+                    new_points = [shapely.Point(x) 
                                 for x in ox.utils_geo.interpolate_points(line, 
                                                                         max_length)]
                     if len(new_points) > 2:
@@ -644,8 +643,8 @@ def __call__(self, G: nx.Graph,
 
         # Get the points for each river and street node
         for u, v, d in G.edges(data=True):
-            upoint = sgeom.Point(G.nodes[u]['x'], G.nodes[u]['y'])
-            vpoint = sgeom.Point(G.nodes[v]['x'], G.nodes[v]['y'])
+            upoint = shapely.Point(G.nodes[u]['x'], G.nodes[u]['y'])
+            vpoint = shapely.Point(G.nodes[v]['x'], G.nodes[v]['y'])
             if d['edge_type'] == 'river':
                 river_points[u] = upoint
                 river_points[v] = vpoint

From 4a77002df379aebdb2d141f2575fd437c1481e21 Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 10:36:50 +0000
Subject: [PATCH 07/17] Remove forward referencing in typing

---
 swmmanywhere/geospatial_utilities.py | 32 ++++++++++++++--------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/swmmanywhere/geospatial_utilities.py b/swmmanywhere/geospatial_utilities.py
index ac1285bc..030a7b3f 100644
--- a/swmmanywhere/geospatial_utilities.py
+++ b/swmmanywhere/geospatial_utilities.py
@@ -340,12 +340,12 @@ def burn_shape_in_raster(geoms: list[sgeom.LineString],
                       nodata = src.nodata) as dest:
             dest.write(data, 1)
 
-def condition_dem(grid: "pysheds.sgrid.Grid", 
-                  dem: "pysheds.sview.Raster") -> "pysheds.sview.Raster":
+def condition_dem(grid: pysheds.sgrid.sGrid, 
+                  dem: pysheds.sview.Raster) -> pysheds.sview.Raster:
     """Condition a DEM with pysheds.
 
     Args:
-        grid (pysheds.sgrid.Grid): The grid object.
+        grid (pysheds.sgrid.sGrid): The grid object.
         dem (pysheds.sview.Raster): The input DEM.
 
     Returns:
@@ -358,13 +358,13 @@ def condition_dem(grid: "pysheds.sgrid.Grid",
 
     return inflated_dem
 
-def compute_flow_directions(grid: "pysheds.sgrid.Grid", 
-                            inflated_dem: "pysheds.sview.Raster") \
-                            -> tuple["pysheds.sview.Raster", tuple]:
+def compute_flow_directions(grid: pysheds.sgrid.sGrid, 
+                            inflated_dem: pysheds.sview.Raster) \
+                            -> tuple[pysheds.sview.Raster, tuple]:
     """Compute flow directions.
 
     Args:
-        grid (pysheds.sgrid.Grid): The grid object.
+        grid (pysheds.sgrid.sGrid): The grid object.
         inflated_dem (pysheds.sview.Raster): The input DEM.
 
     Returns:
@@ -375,13 +375,13 @@ def compute_flow_directions(grid: "pysheds.sgrid.Grid",
     fdir = grid.flowdir(inflated_dem, dirmap=dirmap)
     return fdir, dirmap
 
-def calculate_flow_accumulation(grid: "pysheds.sgrid.Grid", 
-                                fdir: "pysheds.sview.Raster", 
-                                dirmap: tuple) -> "pysheds.sview.Raster":
+def calculate_flow_accumulation(grid: pysheds.sgrid.sGrid, 
+                                fdir: pysheds.sview.Raster, 
+                                dirmap: tuple) -> pysheds.sview.Raster:
     """Calculate flow accumulation.
 
     Args:
-        grid (pysheds.sgrid.Grid): The grid object.
+        grid (pysheds.sgrid.sGrid): The grid object.
         fdir (pysheds.sview.Raster): Flow directions.
         dirmap (tuple): Direction mapping.
 
@@ -391,9 +391,9 @@ def calculate_flow_accumulation(grid: "pysheds.sgrid.Grid",
     acc = grid.accumulation(fdir, dirmap=dirmap)
     return acc
 
-def delineate_catchment(grid: "pysheds.sgrid.Grid",
-                        acc: "pysheds.sview.Raster",
-                        fdir: "pysheds.sview.Raster",
+def delineate_catchment(grid: pysheds.sgrid.sGrid,
+                        acc: pysheds.sview.Raster,
+                        fdir: pysheds.sview.Raster,
                         dirmap: tuple,
                         G: nx.Graph) -> gpd.GeoDataFrame:
     """Delineate catchments.
@@ -502,14 +502,14 @@ def attach_unconnected_subareas(polys_gdf: gpd.GeoDataFrame,
     return polys_gdf
 
 def calculate_slope(polys_gdf: gpd.GeoDataFrame, 
-                    grid: "pysheds.sgrid.Grid", 
+                    grid: pysheds.sgrid.sGrid, 
                     cell_slopes: np.ndarray) -> gpd.GeoDataFrame:
     """Calculate the average slope of each polygon.
 
     Args:
         polys_gdf (gpd.GeoDataFrame): A GeoDataFrame containing polygons with
             columns: 'geometry', 'area', and 'id'. 
-        grid (pysheds.sgrid.Grid): The grid object.
+        grid (pysheds.sgrid.sGrid): The grid object.
         cell_slopes (np.ndarray): The slopes of each cell in the grid.
     
     Returns:

From ed5383fb1a3f3ae1b391d62224c02af44cb86a86 Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 10:39:30 +0000
Subject: [PATCH 08/17] More informative pysheds names

---
 swmmanywhere/geospatial_utilities.py | 32 ++++++++++++++--------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/swmmanywhere/geospatial_utilities.py b/swmmanywhere/geospatial_utilities.py
index 030a7b3f..f1f2eab0 100644
--- a/swmmanywhere/geospatial_utilities.py
+++ b/swmmanywhere/geospatial_utilities.py
@@ -372,36 +372,36 @@ def compute_flow_directions(grid: pysheds.sgrid.sGrid,
         tuple: Direction mapping.
     """
     dirmap = (64, 128, 1, 2, 4, 8, 16, 32)
-    fdir = grid.flowdir(inflated_dem, dirmap=dirmap)
-    return fdir, dirmap
+    flow_dir = grid.flowdir(inflated_dem, dirmap=dirmap)
+    return flow_dir, dirmap
 
 def calculate_flow_accumulation(grid: pysheds.sgrid.sGrid, 
-                                fdir: pysheds.sview.Raster, 
+                                flow_dir: pysheds.sview.Raster, 
                                 dirmap: tuple) -> pysheds.sview.Raster:
     """Calculate flow accumulation.
 
     Args:
         grid (pysheds.sgrid.sGrid): The grid object.
-        fdir (pysheds.sview.Raster): Flow directions.
+        flow_dir (pysheds.sview.Raster): Flow directions.
         dirmap (tuple): Direction mapping.
 
     Returns:
         pysheds.sview.Raster: Flow accumulations.
     """
-    acc = grid.accumulation(fdir, dirmap=dirmap)
-    return acc
+    flow_acc = grid.accumulation(flow_dir, dirmap=dirmap)
+    return flow_acc
 
 def delineate_catchment(grid: pysheds.sgrid.sGrid,
-                        acc: pysheds.sview.Raster,
-                        fdir: pysheds.sview.Raster,
+                        flow_acc: pysheds.sview.Raster,
+                        flow_dir: pysheds.sview.Raster,
                         dirmap: tuple,
                         G: nx.Graph) -> gpd.GeoDataFrame:
     """Delineate catchments.
 
     Args:
         grid (pysheds.sgrid.Grid): The grid object.
-        acc (pysheds.sview.Raster): Flow accumulations.
-        fdir (pysheds.sview.Raster): Flow directions.
+        flow_acc (pysheds.sview.Raster): Flow accumulations.
+        flow_dir (pysheds.sview.Raster): Flow directions.
         dirmap (tuple): Direction mapping.
         G (nx.Graph): The input graph with nodes containing 'x' and 'y'.
     
@@ -416,12 +416,12 @@ def delineate_catchment(grid: pysheds.sgrid.sGrid,
         # Snap the node to the nearest grid cell
         x, y = data['x'], data['y']
         grid_ = deepcopy(grid)
-        x_snap, y_snap = grid_.snap_to_mask(acc > 5, (x, y))
+        x_snap, y_snap = grid_.snap_to_mask(flow_acc > 5, (x, y))
         
         # Delineate the catchment
         catch = grid_.catchment(x=x_snap, 
                                 y=y_snap, 
-                                fdir=fdir, 
+                                fdir=flow_dir, 
                                 dirmap=dirmap, 
                                 xytype='coordinate')
         grid_.clip_to(catch)
@@ -545,16 +545,16 @@ def derive_subcatchments(G: nx.Graph, fid: Path) -> gpd.GeoDataFrame:
     inflated_dem = condition_dem(grid, dem)
 
     # Compute flow directions
-    fdir, dirmap = compute_flow_directions(grid, inflated_dem)
+    flow_dir, dirmap = compute_flow_directions(grid, inflated_dem)
 
     # Calculate slopes
-    cell_slopes = grid.cell_slopes(dem, fdir)
+    cell_slopes = grid.cell_slopes(dem, flow_dir)
 
     # Calculate flow accumulations
-    acc = calculate_flow_accumulation(grid, fdir, dirmap)
+    flow_acc = calculate_flow_accumulation(grid, flow_dir, dirmap)
 
     # Delineate catchments
-    polys = delineate_catchment(grid, acc, fdir, dirmap, G)
+    polys = delineate_catchment(grid, flow_acc, flow_dir, dirmap, G)
 
     # Remove intersections
     result_polygons = remove_intersections(polys)

From 6dd739468ed12727d0156dba933c60050e38fe15 Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 10:41:31 +0000
Subject: [PATCH 09/17] Remove completed TODO

---
 swmmanywhere/geospatial_utilities.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/swmmanywhere/geospatial_utilities.py b/swmmanywhere/geospatial_utilities.py
index f1f2eab0..976d22c7 100644
--- a/swmmanywhere/geospatial_utilities.py
+++ b/swmmanywhere/geospatial_utilities.py
@@ -409,7 +409,6 @@ def delineate_catchment(grid: pysheds.sgrid.sGrid,
         gpd.GeoDataFrame: A GeoDataFrame containing polygons with columns:
             'geometry', 'area', and 'id'. Sorted by area in descending order.
     """
-    #TODO - rather than using this mad list of dicts better to just use a gdf
     polys = []
     # Iterate over the nodes in the graph
     for id, data in tqdm(G.nodes(data=True), total=len(G.nodes)):

From f06043b4a6108a3fc74a19a8cdb35027a5691080 Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 20 Feb 2024 10:42:27 +0000
Subject: [PATCH 10/17] Update swmmanywhere/geospatial_utilities.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
---
 swmmanywhere/geospatial_utilities.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/swmmanywhere/geospatial_utilities.py b/swmmanywhere/geospatial_utilities.py
index 976d22c7..11e4953e 100644
--- a/swmmanywhere/geospatial_utilities.py
+++ b/swmmanywhere/geospatial_utilities.py
@@ -468,14 +468,14 @@ def remove_zero_area_subareas(mp: sgeom.MultiPolygon,
     Returns:
         sgeom.MultiPolygon: A multipolygon with zero area subareas removed.
     """
-    if hasattr(mp, 'geoms'):
-        largest = max(mp.geoms, key=lambda x: x.area)
-        removed = [subarea for subarea in mp.geoms if subarea != largest]
-        removed_subareas.extend(removed)
-        return largest
-    else:
+    if not hasattr(mp, 'geoms'):
         return mp
 
+    largest = max(mp.geoms, key=lambda x: x.area)
+    removed = [subarea for subarea in mp.geoms if subarea != largest]
+    removed_subareas.extend(removed)
+    return largest
+
 def attach_unconnected_subareas(polys_gdf: gpd.GeoDataFrame, 
                                 unconnected_subareas: List[sgeom.Polygon]) \
                                     -> gpd.GeoDataFrame:

From 3b5e1f7d99fdf4f77160ac5e10975e4b3d2442d9 Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 10:43:49 +0000
Subject: [PATCH 11/17] move serialize linestring out of save_graph

---
 swmmanywhere/graph_utilities.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 5cfe9977..2e2d404d 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -42,7 +42,11 @@ def load_graph(fid: Path) -> nx.Graph:
             line_string = shapely.LineString(shapely.wkt.loads(geometry_coords))
             data['geometry'] = line_string
     return G
-
+def _serialize_line_string(obj):
+    if isinstance(obj, shapely.LineString):
+        return obj.wkt
+    else:
+        return obj
 def save_graph(G: nx.Graph, 
                fid: Path) -> None:
     """Save a graph to a file.
@@ -52,15 +56,11 @@ def save_graph(G: nx.Graph,
         fid (Path): The path to the file
     """
     json_data = nx.node_link_data(G)
-    def serialize_line_string(obj):
-        if isinstance(obj, shapely.LineString):
-            return obj.wkt
-        else:
-            return obj
+    
     with open(fid, 'w') as file:
         json.dump(json_data, 
                   file,
-                  default = serialize_line_string)
+                  default = _serialize_line_string)
 
 
 class BaseGraphFunction(ABC):

From 6cba347a8153a55a9185f694cdad5ed7f0a711e2 Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 20 Feb 2024 10:44:38 +0000
Subject: [PATCH 12/17] Update swmmanywhere/graph_utilities.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
---
 swmmanywhere/graph_utilities.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 5cfe9977..215350b0 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -95,10 +95,10 @@ def validate_requirements(self,
                               node_attributes: set) -> None:
         """Validate that the graph has the required attributes."""
         for attribute in self.required_edge_attributes:
-            assert attribute in edge_attributes, f"{attribute} not in attributes"
+            assert attribute in edge_attributes, f"{attribute} not in edge attributes"
             
         for attribute in self.required_node_attributes:
-            assert attribute in node_attributes, f"{attribute} not in attributes"
+            assert attribute in node_attributes, f"{attribute} not in node attributes"
             
 
     def add_graphfcn(self,

From 19707a00df738176b473bb78c6bfcc121154c515 Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 11:38:28 +0000
Subject: [PATCH 13/17] Use __init_subclass__ for graphfcn

---
 swmmanywhere/graph_utilities.py | 197 +++++++++++++-------------------
 1 file changed, 81 insertions(+), 116 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 47ce20b2..9b7d2a54 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -10,7 +10,7 @@
 from heapq import heappop, heappush
 from itertools import product
 from pathlib import Path
-from typing import Any, Callable, Dict, Hashable, List
+from typing import Any, Callable, Dict, Hashable, List, Optional
 
 import geopandas as gpd
 import networkx as nx
@@ -64,23 +64,34 @@ def save_graph(G: nx.Graph,
 
 
 class BaseGraphFunction(ABC):
-    """Base class for graph functions."""
-
-    @abstractmethod
-    def __init__(self):
-        """Initialize the class.
-        
-        On a SWMManywhere project the intention is to iterate over a number of
-        graph functions. Each graph function may require certain attributes to
-        be present in the graph. Each graph function may add attributes to the
-        graph. This class provides a framework for graph functions to check
-        their requirements and additions a-priori when the list is provided.
-        """
-        #TODO just attribute name is fine - or type too...
-        self.required_edge_attributes = []
-        self.adds_edge_attributes = []
-        self.required_node_attributes = []
-        self.adds_node_attributes = []
+    """Base class for graph functions.
+    
+    On a SWMManywhere project the intention is to iterate over a number of
+    graph functions. Each graph function may require certain attributes to
+    be present in the graph. Each graph function may add attributes to the
+    graph. This class provides a framework for graph functions to check
+    their requirements and additions a-priori when the list is provided.
+    """
+    
+    required_edge_attributes: List[str] = list()
+    adds_edge_attributes: List[str] = list()
+    required_node_attributes: List[str] = list()
+    adds_node_attributes: List[str] = list()
+    def __init_subclass__(cls, 
+                          required_edge_attributes: Optional[List[str]] = None,
+                          adds_edge_attributes: Optional[List[str]] = None,
+                          required_node_attributes : Optional[List[str]] = None,
+                          adds_node_attributes : Optional[List[str]] = None
+                          ):
+        """Set the required and added attributes for the subclass."""
+        cls.required_edge_attributes = required_edge_attributes if \
+            required_edge_attributes else []
+        cls.adds_edge_attributes = adds_edge_attributes if \
+            adds_edge_attributes  else []
+        cls.required_node_attributes = required_node_attributes if \
+            required_node_attributes else []
+        cls.adds_node_attributes = adds_node_attributes if \
+            adds_node_attributes  else []
 
     @abstractmethod
     def __call__(self, 
@@ -136,13 +147,11 @@ def get_osmid_id(data: dict) -> Hashable:
     return id_
 
 @register_graphfcn
-class assign_id(BaseGraphFunction):
+class assign_id(BaseGraphFunction, 
+                required_edge_attributes = ['osmid'], 
+                adds_edge_attributes = ['id']
+                ):
     """assign_id class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_edge_attributes = ['osmid']
-        self.adds_edge_attributes = ['id']
 
     def __call__(self,
                  G: nx.Graph, 
@@ -165,17 +174,12 @@ def __call__(self,
         return G
 
 @register_graphfcn
-class format_osmnx_lanes(BaseGraphFunction):
+class format_osmnx_lanes(BaseGraphFunction,
+                         required_edge_attributes = ['lanes'],
+                         adds_edge_attributes = ['width']):
     """format_osmnx_lanes class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        
-        # i.e., in osmnx format, i.e., empty for single lane, an int for a
-        # number of lanes or a list if the edge has multiple carriageways
-        self.required_edge_attributes = ['lanes']
-        
-        self.adds_edge_attributes = ['width']
+    # i.e., in osmnx format, i.e., empty for single lane, an int for a
+    # number of lanes or a list if the edge has multiple carriageways
 
     def __call__(self,
                  G: nx.Graph, 
@@ -203,12 +207,10 @@ def __call__(self,
         return G
 
 @register_graphfcn
-class double_directed(BaseGraphFunction):
+class double_directed(BaseGraphFunction,
+                      required_edge_attributes = ['id']):
     """double_directed class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_edge_attributes = ['id']
+    
     def __call__(self, G: nx.Graph, **kwargs) -> nx.Graph:
         """Create a 'double directed graph'.
 
@@ -238,13 +240,10 @@ def __call__(self, G: nx.Graph, **kwargs) -> nx.Graph:
         return G_new
 
 @register_graphfcn
-class split_long_edges(BaseGraphFunction):
+class split_long_edges(BaseGraphFunction,
+                       required_edge_attributes = ['id', 'geometry', 'length']):
     """split_long_edges class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_edge_attributes = ['id', 'geometry', 'length']
-
+    
     def __call__(self, 
                  G: nx.Graph, 
                  subcatchment_derivation: parameters.SubcatchmentDerivation, 
@@ -254,12 +253,8 @@ def __call__(self,
         This function splits long edges into shorter edges. The edges are split
         into segments of length 'max_street_length'. The first and last segment
         are connected to the original nodes. Intermediate segments are connected
-        to newly created nodes.
-
-        Requires a graph with edges that have:
-            - 'geometry' (shapely LineString)
-            - 'length' (float)
-            - 'id' (str)
+        to newly created nodes. The 'geometry' of the original edge must be
+        a LineString.
         
         Args:
             G (nx.Graph): A graph
@@ -369,15 +364,12 @@ def create_new_edge_data(line, data, id_):
         return graph
 
 @register_graphfcn
-class calculate_contributing_area(BaseGraphFunction):
+class calculate_contributing_area(BaseGraphFunction,
+                                required_edge_attributes = ['id', 'geometry', 'width'],
+                                adds_edge_attributes = ['contributing_area'],
+                                adds_node_attributes = ['contributing_area']):
     """calculate_contributing_area class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_edge_attributes = ['id', 'geometry', 'width']
-        self.adds_edge_attributes = ['contributing_area']
-        self.adds_node_attributes = ['contributing_area']
-
+    
     def __call__(self, G: nx.Graph, 
                          subcatchment_derivation: parameters.SubcatchmentDerivation,
                          addresses: parameters.FilePaths,
@@ -439,14 +431,11 @@ def __call__(self, G: nx.Graph,
         return G
 
 @register_graphfcn
-class set_elevation(BaseGraphFunction):
+class set_elevation(BaseGraphFunction,
+                    required_node_attributes = ['x', 'y'],
+                    adds_node_attributes = ['elevation']):
     """set_elevation class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_node_attributes = ['x', 'y']
-        self.adds_node_attributes = ['elevation']
-
+    
     def __call__(self, G: nx.Graph, 
                   addresses: parameters.FilePaths,
                   **kwargs) -> nx.Graph:
@@ -474,13 +463,10 @@ def __call__(self, G: nx.Graph,
         return G
 
 @register_graphfcn
-class set_surface_slope(BaseGraphFunction):
+class set_surface_slope(BaseGraphFunction,
+                        required_node_attributes = ['elevation'],
+                        adds_edge_attributes = ['surface_slope']):
     """set_surface_slope class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_node_attributes = ['elevation']
-        self.adds_edge_attributes = ['surface_slope']
 
     def __call__(self, G: nx.Graph,
                       **kwargs) -> nx.Graph:
@@ -507,13 +493,10 @@ def __call__(self, G: nx.Graph,
         return G
 
 @register_graphfcn
-class set_chahinan_angle(BaseGraphFunction):
+class set_chahinan_angle(BaseGraphFunction,
+                         required_node_attributes = ['x','y'],
+                         adds_edge_attributes = ['chahinan_angle']):
     """set_chahinan_angle class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_node_attributes = ['x','y']
-        self.adds_edge_attributes = ['chahinan_angle']
 
     def __call__(self, G: nx.Graph, 
                        **kwargs) -> nx.Graph:
@@ -552,15 +535,13 @@ def __call__(self, G: nx.Graph,
         return G
 
 @register_graphfcn
-class calculate_weights(BaseGraphFunction):
+class calculate_weights(BaseGraphFunction,
+                        required_edge_attributes = 
+                        parameters.TopologyDerivation().weights,
+                        adds_edge_attributes = ['weight']):
     """calculate_weights class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        # TODO.. I guess if someone defines their own weights, this will need 
-        # to change, will want an automatic way to do that...
-        self.required_attributes = parameters.TopologyDerivation().weights
-        self.adds_edge_attributes = ['weight']
+    # TODO.. I guess if someone defines their own weights, this will need 
+    # to change, will want an automatic way to do that...
 
     def __call__(self, G: nx.Graph, 
                        topo_derivation: parameters.TopologyDerivation,
@@ -604,13 +585,10 @@ def __call__(self, G: nx.Graph,
         return G
 
 @register_graphfcn
-class identify_outlets(BaseGraphFunction):
+class identify_outlets(BaseGraphFunction,
+                       required_edge_attributes = ['length', 'edge_type'],
+                       required_node_attributes = ['x', 'y']):
     """identify_outlets class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_edge_attributes = ['length', 'edge_type']
-        self.required_node_attributes = ['x', 'y']
 
     def __call__(self, G: nx.Graph,
                      outlet_derivation: parameters.OutletDerivation,
@@ -619,14 +597,6 @@ def __call__(self, G: nx.Graph,
 
         This function identifies outlets in a combined river-street graph. An
         outlet is a node that is connected to a river and a street. 
-        
-        # TODO an automatic way to handle something like this? maybe 
-        # required_graph_attributes = ['outlets'] or something
-
-        Adds new edges to represent outlets with the attributes:
-            - 'edge_type' ('outlet')
-            - 'length' (float)
-            - 'id' (str)
 
         Args:
             G (nx.Graph): A graph
@@ -699,14 +669,12 @@ def __call__(self, G: nx.Graph,
         return G
 
 @register_graphfcn
-class derive_topology(BaseGraphFunction):
+class derive_topology(BaseGraphFunction,
+                      required_edge_attributes = ['edge_type', # 'rivers' and 'streets'
+                                                  'weight'],
+                      adds_node_attributes = ['outlet', 'shortest_path']):
     """derive_topology class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        # both 'rivers' and 'streets' in 'edge_type'
-        self.required_edge_attributes = ['edge_type', 'weight'] 
-        self.adds_node_attributes = ['outlet', 'shortest_path']
+    
 
     def __call__(self, G: nx.Graph,
                     **kwargs) -> nx.Graph:
@@ -941,16 +909,13 @@ def process_successors(G: nx.Graph,
                 to derive topology''')
 
 @register_graphfcn
-class pipe_by_pipe(BaseGraphFunction):
+class pipe_by_pipe(BaseGraphFunction,
+                   required_edge_attributes = ['length', 'elevation'],
+                   required_node_attributes = ['contributing_area', 'elevation'],
+                   adds_edge_attributes = ['diameter'],
+                   adds_node_attributes = ['chamber_floor_elevation']):
     """pipe_by_pipe class."""
-    def __init__(self):
-        """Initialize the class."""
-        super().__init__()
-        self.required_edge_attributes = ['length', 'elevation']
-        self.required_node_attributes = ['contributing_area', 'elevation']
-        self.adds_edge_attributes = ['diameter']
-        self.adds_node_attributes = ['chamber_floor_elevation']
-        # If doing required_graph_attributes - it would be something like 'dendritic'
+    # If doing required_graph_attributes - it would be something like 'dendritic'
 
     def __call__(self, 
                  G: nx.Graph, 

From ff8cbc4256c4e52bc39c14351969eb7408e4ab6c Mon Sep 17 00:00:00 2001
From: barneydobson <barnaby.dobson1@gmail.com>
Date: Tue, 20 Feb 2024 11:42:08 +0000
Subject: [PATCH 14/17] Update swmmanywhere/graph_utilities.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
---
 swmmanywhere/graph_utilities.py | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 9b7d2a54..40a4833f 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -519,16 +519,18 @@ def __call__(self, G: nx.Graph,
         for u,v,d in G.edges(data=True):
             min_weight = float('inf')
             for node in G.successors(v):
-                if node != u:
-                    p1 = (G.nodes[u]['x'], G.nodes[u]['y'])
-                    p2 = (G.nodes[v]['x'], G.nodes[v]['y'])
-                    p3 = (G.nodes[node]['x'], G.nodes[node]['y'])
-                    angle = go.calculate_angle(p1,p2,p3)
-                    chahinan_weight = np.interp(angle,
-                                                [0, 90, 135, 180, 225, 270, 360],
-                                                [1, 0.2, 0.7, 0, 0.7, 0.2, 1]
-                                                )
-                    min_weight = min(chahinan_weight, min_weight)
+                if node == u:
+                    continue
+                    
+                p1 = (G.nodes[u]['x'], G.nodes[u]['y'])
+                p2 = (G.nodes[v]['x'], G.nodes[v]['y'])
+                p3 = (G.nodes[node]['x'], G.nodes[node]['y'])
+                angle = go.calculate_angle(p1,p2,p3)
+                chahinan_weight = np.interp(angle,
+                                            [0, 90, 135, 180, 225, 270, 360],
+                                            [1, 0.2, 0.7, 0, 0.7, 0.2, 1]
+                                            )
+                min_weight = min(chahinan_weight, min_weight)
             if min_weight == float('inf'):
                 min_weight = 0
             d['chahinan_angle'] = min_weight

From beb0b76d1494ebd75b3d5c9d8de3e5dc865e303f Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 11:44:14 +0000
Subject: [PATCH 15/17] Test opposite and unindent in shortest path

---
 swmmanywhere/graph_utilities.py | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index 40a4833f..a2809a1b 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -737,13 +737,15 @@ def __call__(self, G: nx.Graph,
                 alt_dist = dist + edge_data['weight']
                 # If the alternative distance is shorter
 
-                if alt_dist < shortest_paths[neighbor]:
-                    # Update the shortest path length
-                    shortest_paths[neighbor] = alt_dist
-                    # Update the path
-                    paths[neighbor] = paths[node] + [neighbor]
-                    # Push the neighbor to the heap
-                    heappush(heap, (alt_dist, neighbor))
+                if alt_dist >= shortest_paths[neighbor]:
+                    continue
+                
+                # Update the shortest path length
+                shortest_paths[neighbor] = alt_dist
+                # Update the path
+                paths[neighbor] = paths[node] + [neighbor]
+                # Push the neighbor to the heap
+                heappush(heap, (alt_dist, neighbor))
 
         edges_to_keep = set()
         for path in paths.values():

From 4a1cca99bfa1b1891c3c7e85c4107c760d5b740f Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 11:47:57 +0000
Subject: [PATCH 16/17] Add negative cycle check

---
 swmmanywhere/graph_utilities.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index a2809a1b..e1e86b57 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -714,6 +714,10 @@ def __call__(self, G: nx.Graph,
         for u in set(nodes_to_remove).union(isolated_nodes):
             G.remove_node(u)
 
+        # Check for negative cycles
+        if nx.negative_edge_cycle(G):
+            raise ValueError('Graph contains negative cycle')
+
         # Initialize the dictionary with infinity for all nodes
         shortest_paths = {node: float('inf') for node in G.nodes}
 

From 2bfedafb43b247286e63e33bc0ca4d3542ac46b9 Mon Sep 17 00:00:00 2001
From: Dobson <bdobson@ic.ac.uk>
Date: Tue, 20 Feb 2024 11:49:19 +0000
Subject: [PATCH 17/17] Add weighted negative cycle check

---
 swmmanywhere/graph_utilities.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/swmmanywhere/graph_utilities.py b/swmmanywhere/graph_utilities.py
index e1e86b57..962b55c6 100644
--- a/swmmanywhere/graph_utilities.py
+++ b/swmmanywhere/graph_utilities.py
@@ -715,7 +715,7 @@ def __call__(self, G: nx.Graph,
             G.remove_node(u)
 
         # Check for negative cycles
-        if nx.negative_edge_cycle(G):
+        if nx.negative_edge_cycle(G, weight = 'weight'):
             raise ValueError('Graph contains negative cycle')
 
         # Initialize the dictionary with infinity for all nodes