From a781f225ae35ef248b0ddc8e6e9ce5305048fb75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Tue, 10 Dec 2024 14:12:37 +0100 Subject: [PATCH 01/17] chore: Removed intermediate export logic --- ...ptimal_route_origin_closest_destination.py | 81 +++++-------------- 1 file changed, 19 insertions(+), 62 deletions(-) diff --git a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py index 43096574a..b5fc399f5 100644 --- a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py +++ b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py @@ -8,6 +8,7 @@ AnalysisSectionLosses, ) from ra2ce.analysis.analysis_input_wrapper import AnalysisInputWrapper +from ra2ce.analysis.analysis_result.analysis_result import AnalysisResult from ra2ce.analysis.analysis_result.analysis_result_wrapper import AnalysisResultWrapper from ra2ce.analysis.losses.analysis_losses_protocol import AnalysisLossesProtocol from ra2ce.analysis.losses.origin_closest_destination import OriginClosestDestination @@ -44,74 +45,30 @@ def __init__( self.file_id = analysis_input.file_id self._analysis_input = analysis_input - def _save_gdf(self, gdf: GeoDataFrame, save_path: Path): - """Takes in a geodataframe object and outputs shapefiles at the paths indicated by edge_shp and node_shp - - Arguments: - gdf [geodataframe]: geodataframe object to be converted - save_path [str]: output path including extension for edges shapefile - Returns: - None - """ - # save to shapefile - gdf.crs = "epsg:4326" # TODO: decide if this should be variable with e.g. an output_crs configured - - for col in gdf.columns: - if gdf[col].dtype == object and col != gdf.geometry.name: - gdf[col] = gdf[col].astype(str) - - if save_path.exists(): - save_path.unlink() - gdf.to_file(save_path, driver="GPKG") - logging.info("Results saved to: {}".format(save_path)) - def execute(self) -> AnalysisResultWrapper: - def _save_gpkg_analysis( - base_graph, - to_save_gdf: list[GeoDataFrame], - to_save_gdf_names: list[str], - ): - for to_save, save_name in zip(to_save_gdf, to_save_gdf_names): - if not to_save.empty: - gpkg_path = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + f"_{save_name}.gpkg" - ) - self._save_gdf(to_save, gpkg_path) - - # Save the Graph - gpkg_path_nodes = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_results_nodes.gpkg" - ) - gpkg_path_edges = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_results_edges.gpkg" - ) - graph_to_gpkg(base_graph, gpkg_path_edges, gpkg_path_nodes) - - _output_path = self.output_path.joinpath(self.analysis.analysis.config_value) - analyzer = OriginClosestDestination(self._analysis_input) ( - base_graph, + _, opt_routes, destinations, ) = analyzer.optimal_route_origin_closest_destination() - if self.analysis.save_gpkg: - # Save the GeoDataFrames - to_save_gdf = [destinations, opt_routes] - to_save_gdf_names = ["destinations", "optimal_routes"] - _save_gpkg_analysis(base_graph, to_save_gdf, to_save_gdf_names) - if self.analysis.save_csv: - csv_path = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_destinations.csv" + # TODO: This does not seem correct, why were we returning None? + def get_analysis_result( + gdf_result: GeoDataFrame, suffix_name: str + ) -> AnalysisResult: + _ar = AnalysisResult( + analysis_result=gdf_result, + analysis_config=self.analysis, + output_path=self.output_path, ) - del destinations["geometry"] - destinations.to_csv(csv_path, index=False) + _ar.analysis_name = self.analysis.name.replace(" ", "_") + suffix_name + return _ar - csv_path = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_optimal_routes.csv" - ) - del opt_routes["geometry"] - opt_routes.to_csv(csv_path, index=False) - # TODO: This does not seem correct, why were we returning None? - return self.generate_result_wrapper(None) + return AnalysisResultWrapper( + results_collection=[ + # CSV + get_analysis_result(destinations, "_destinations"), + get_analysis_result(opt_routes, "_optimal_routes"), + ] + ) From de63defc2172f85e4f253040e04224c7033e805b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Tue, 10 Dec 2024 14:42:53 +0100 Subject: [PATCH 02/17] feat: Refactored both analyses to generate an `AnalysisResultWrapper` --- .../multi_link_origin_closest_destination.py | 104 ++++-------------- ...ptimal_route_origin_closest_destination.py | 21 ++-- 2 files changed, 34 insertions(+), 91 deletions(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index b9d0a9010..69830c095 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -7,6 +7,7 @@ AnalysisSectionLosses, ) from ra2ce.analysis.analysis_input_wrapper import AnalysisInputWrapper +from ra2ce.analysis.analysis_result.analysis_result import AnalysisResult from ra2ce.analysis.analysis_result.analysis_result_wrapper import AnalysisResultWrapper from ra2ce.analysis.losses.analysis_losses_protocol import AnalysisLossesProtocol from ra2ce.analysis.losses.origin_closest_destination import OriginClosestDestination @@ -15,8 +16,6 @@ from ra2ce.network.network_config_data.network_config_data import ( OriginsDestinationsSection, ) -from ra2ce.network.networks_utils import graph_to_gpkg -from ra2ce.ra2ce_logger import logging class MultiLinkOriginClosestDestination(AnalysisBase, AnalysisLossesProtocol): @@ -46,56 +45,24 @@ def __init__( self.file_id = analysis_input.file_id self._analysis_input = analysis_input - def _save_gdf(self, gdf: GeoDataFrame, save_path: Path) -> None: - """Takes in a geodataframe object and outputs shapefiles at the paths indicated by edge_shp and node_shp - - Arguments: - gdf [geodataframe]: geodataframe object to be converted - save_path [str]: output path including extension for edges shapefile - Returns: - None - """ - # save to shapefile - gdf.crs = "epsg:4326" # TODO: decide if this should be variable with e.g. an output_crs configured - - for col in gdf.columns: - if gdf[col].dtype == object and col != gdf.geometry.name: - gdf[col] = gdf[col].astype(str) - - if save_path.exists(): - save_path.unlink() - gdf.to_file(save_path, driver="GPKG") - logging.info("Results saved to: {}".format(save_path)) - def execute(self) -> AnalysisResultWrapper: - def _save_gpkg_analysis( - base_graph, - to_save_gdf: list[GeoDataFrame], - to_save_gdf_names: list[str], - ): - for to_save, save_name in zip(to_save_gdf, to_save_gdf_names): - if not to_save.empty: - gpkg_path = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + f"_{save_name}.gpkg" - ) - self._save_gdf(to_save, gpkg_path) - - # Save the Graph - gpkg_path_nodes = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_results_nodes.gpkg" + def get_analysis_result( + gdf_result: GeoDataFrame, suffix_name: str + ) -> AnalysisResult: + _ar = AnalysisResult( + analysis_result=gdf_result, + analysis_config=self.analysis, + output_path=self.output_path, ) - gpkg_path_edges = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_results_edges.gpkg" - ) - graph_to_gpkg(base_graph, gpkg_path_edges, gpkg_path_nodes) + _ar.analysis_name = self.analysis.name.replace(" ", "_") + suffix_name + return _ar _output_path = self.output_path.joinpath(self.analysis.analysis.config_value) analyzer = OriginClosestDestination(self._analysis_input) - if self.analysis.calculate_route_without_disruption: ( - base_graph, + _, opt_routes_without_hazard, destinations, ) = analyzer.optimal_route_origin_closest_destination() @@ -119,7 +86,7 @@ def _save_gpkg_analysis( ) else: ( - base_graph, + _, origins, destinations, agg_results, @@ -127,40 +94,18 @@ def _save_gpkg_analysis( ) = analyzer.multi_link_origin_closest_destination() opt_routes_without_hazard = GeoDataFrame() - if self.analysis.save_gpkg: - # Save the GeoDataFrames - to_save_gdf = [ - origins, - destinations, - opt_routes_without_hazard, - opt_routes_with_hazard, - ] - to_save_gdf_names = [ - "origins", - "destinations", - "optimal_routes_without_hazard", - "optimal_routes_with_hazard", + _analysis_result_wrapper = AnalysisResultWrapper( + results_collection=[ + get_analysis_result(origins, "_origins"), + get_analysis_result(destinations, "_destinations"), + get_analysis_result( + opt_routes_without_hazard, "_optimal_routes_without_hazard" + ), + get_analysis_result( + opt_routes_with_hazard, "_optimal_routes_with_hazard" + ), ] - _save_gpkg_analysis(base_graph, to_save_gdf, to_save_gdf_names) - if self.analysis.save_csv: - csv_path = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_destinations.csv" - ) - if "geometry" in destinations.columns: - del destinations["geometry"] - if not csv_path.parent.exists(): - csv_path.parent.mkdir(parents=True) - destinations.to_csv(csv_path, index=False) - - csv_path = _output_path.joinpath( - self.analysis.name.replace(" ", "_") + "_optimal_routes.csv" - ) - if not opt_routes_without_hazard.empty: - del opt_routes_without_hazard["geometry"] - opt_routes_without_hazard.to_csv(csv_path, index=False) - if not opt_routes_with_hazard.empty: - del opt_routes_with_hazard["geometry"] - opt_routes_with_hazard.to_csv(csv_path, index=False) + ) if self.graph_file_hazard.file is not None: agg_results.to_excel( @@ -170,5 +115,4 @@ def _save_gpkg_analysis( index=False, ) - # TODO: This does not seem correct, why were we returning None? - return self.generate_result_wrapper(None) + return _analysis_result_wrapper diff --git a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py index b5fc399f5..c1429fbc3 100644 --- a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py +++ b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py @@ -1,4 +1,3 @@ -import logging from pathlib import Path from geopandas import GeoDataFrame @@ -17,7 +16,6 @@ from ra2ce.network.network_config_data.network_config_data import ( OriginsDestinationsSection, ) -from ra2ce.network.networks_utils import graph_to_gpkg class OptimalRouteOriginClosestDestination(AnalysisBase, AnalysisLossesProtocol): @@ -46,14 +44,6 @@ def __init__( self._analysis_input = analysis_input def execute(self) -> AnalysisResultWrapper: - analyzer = OriginClosestDestination(self._analysis_input) - ( - _, - opt_routes, - destinations, - ) = analyzer.optimal_route_origin_closest_destination() - - # TODO: This does not seem correct, why were we returning None? def get_analysis_result( gdf_result: GeoDataFrame, suffix_name: str ) -> AnalysisResult: @@ -65,9 +55,18 @@ def get_analysis_result( _ar.analysis_name = self.analysis.name.replace(" ", "_") + suffix_name return _ar + analyzer = OriginClosestDestination(self._analysis_input) + + # Get gdfs + ( + base_graph, + opt_routes, + destinations, + ) = analyzer.optimal_route_origin_closest_destination() + return AnalysisResultWrapper( results_collection=[ - # CSV + get_analysis_result(base_graph, "_origins"), get_analysis_result(destinations, "_destinations"), get_analysis_result(opt_routes, "_optimal_routes"), ] From 58131f2cfb450b7655b2d6d1de975869c349802c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Tue, 10 Dec 2024 16:35:22 +0100 Subject: [PATCH 03/17] chore: Corrected network option to get back the nodes and edges whilst not exporting --- .../multi_link_origin_closest_destination.py | 8 ++- ra2ce/network/networks_utils.py | 57 ++++++++++++++----- tests/test_main.py | 4 +- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index 69830c095..feb63c4f8 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -16,6 +16,7 @@ from ra2ce.network.network_config_data.network_config_data import ( OriginsDestinationsSection, ) +from ra2ce.network.networks_utils import get_nodes_and_edges_from_origin_graph class MultiLinkOriginClosestDestination(AnalysisBase, AnalysisLossesProtocol): @@ -62,7 +63,7 @@ def get_analysis_result( analyzer = OriginClosestDestination(self._analysis_input) if self.analysis.calculate_route_without_disruption: ( - _, + _base_graph, opt_routes_without_hazard, destinations, ) = analyzer.optimal_route_origin_closest_destination() @@ -86,7 +87,7 @@ def get_analysis_result( ) else: ( - _, + _base_graph, origins, destinations, agg_results, @@ -94,6 +95,7 @@ def get_analysis_result( ) = analyzer.multi_link_origin_closest_destination() opt_routes_without_hazard = GeoDataFrame() + _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(base_graph) _analysis_result_wrapper = AnalysisResultWrapper( results_collection=[ get_analysis_result(origins, "_origins"), @@ -104,6 +106,8 @@ def get_analysis_result( get_analysis_result( opt_routes_with_hazard, "_optimal_routes_with_hazard" ), + get_analysis_result(_nodes_graph, "_results_nodes"), + get_analysis_result(_edges_graph, "_results_edges"), ] ) diff --git a/ra2ce/network/networks_utils.py b/ra2ce/network/networks_utils.py index 0e9a76f54..a508bad0d 100644 --- a/ra2ce/network/networks_utils.py +++ b/ra2ce/network/networks_utils.py @@ -1142,47 +1142,74 @@ def graph_to_gdf( return edges, nodes -def graph_to_gpkg(origin_graph: nx.Graph, edge_gpkg: str, node_gpkg: str) -> None: - """Takes in a networkx graph object and outputs shapefiles at the paths indicated by edge_gpkg and node_gpkg +def get_nodes_and_edges_from_origin_graph( + origin_graph: nx.Graph, +) -> tuple[gpd.GeoDataFrame, gpd.GeoDataFrame]: + """ + Takes in a networkx graph object and returns the `GeoDataFrame` with separated + nodes and edges. Arguments: origin_graph [nx.Graph]: networkx graph object to be converted - edge_gpkg [str]: output path including extension for edges geopackage - node_gpkg [str]: output path including extension for nodes geopackage Returns: - None + tuple[gpd.GeoDataFrame, gpd.GeoDataFrame]: + resulting tuple of formatted `gpd.GeoDataFrame` for nodes and edges. """ # now only multidigraphs and graphs are used - if type(origin_graph) == nx.Graph: + if isinstance(origin_graph, nx.Graph): origin_graph = nx.MultiGraph(origin_graph) # The nodes should have a geometry attribute (perhaps on top of the x and y attributes) - nodes, edges = graph_to_gdfs(origin_graph, node_geometry=False) + _nodes, _edges = graph_to_gdfs(origin_graph, node_geometry=False) - dfs = [edges, nodes] + dfs = [_edges, _nodes] for df in dfs: for col in df.columns: if df[col].dtype == object and col != df.geometry.name: df[col] = df[col].astype(str) # Add a CRS to the nodes - if nodes.crs is None and edges.crs is not None: - nodes.crs = edges.crs + if _nodes.crs is None and _edges.crs is not None: + _nodes.crs = _edges.crs + + # The encoding utf-8 might result in an empty shapefile if the wrong encoding is used. + for entity in [_nodes, _edges]: + if "osmid" in entity: + # Otherwise it gives this error: cannot insert osmid, already exist + entity["osmid_original"] = entity.pop("osmid") + + return _nodes, _edges + + +def graph_to_gpkg(origin_graph: nx.Graph, edge_gpkg: str, node_gpkg: str) -> None: + """ + [DEPRECATED] Please use instead `AnalysisResultWrapperExporter`. + Takes in a networkx graph object and outputs shapefiles at the paths indicated by edge_gpkg and node_gpkg + + Arguments: + origin_graph [nx.Graph]: networkx graph object to be converted + edge_gpkg [str]: output path including extension for edges geopackage + node_gpkg [str]: output path including extension for nodes geopackage + + Returns: + None + """ + _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(origin_graph) - logging.info("Saving nodes as shapefile: {}".format(node_gpkg)) - logging.info("Saving edges as shapefile: {}".format(edge_gpkg)) + logging.info("Saving nodes as shapefile: %s".format(node_gpkg)) + logging.info("Saving edges as shapefile: %s".format(edge_gpkg)) # The encoding utf-8 might result in an empty shapefile if the wrong encoding is used. - for entity in [nodes, edges]: + for entity in [_nodes_graph, _edges_graph]: if "osmid" in entity: # Otherwise it gives this error: cannot insert osmid, already exist entity["osmid_original"] = entity.pop("osmid") for _path in [node_gpkg, edge_gpkg]: if _path.exists(): _path.unlink() - nodes.to_file(node_gpkg, driver="GPKG", encoding="utf-8") - edges.to_file(edge_gpkg, driver="GPKG", encoding="utf-8") + _nodes_graph.to_file(node_gpkg, driver="GPKG", encoding="utf-8") + _edges_graph.to_file(edge_gpkg, driver="GPKG", encoding="utf-8") @staticmethod diff --git a/tests/test_main.py b/tests/test_main.py index 92b583ffc..6b99a9509 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -235,7 +235,9 @@ def _verify_file(filepath: Path) -> bool: list( chain( *( - list(map(lambda x: _verify_file(_analysis_dir / k / x), v)) + list( + map(lambda x: _verify_file(_analysis_dir.joinpath(k, x)), v) + ) for k, v in expected_analysis_files.items() ) ) From b3cf997cd53ab7305d3bec5d3e320e85a2bba337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Tue, 10 Dec 2024 16:48:45 +0100 Subject: [PATCH 04/17] chore: Small correction to output file --- .../multi_link_origin_closest_destination.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index feb63c4f8..8df3d6e97 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -73,7 +73,7 @@ def get_analysis_result( opt_routes_with_hazard = GeoDataFrame(data=None) else: ( - base_graph, + _base_graph, origins, destinations, agg_results, @@ -95,21 +95,24 @@ def get_analysis_result( ) = analyzer.multi_link_origin_closest_destination() opt_routes_without_hazard = GeoDataFrame() - _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(base_graph) + _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(_base_graph) _analysis_result_wrapper = AnalysisResultWrapper( results_collection=[ get_analysis_result(origins, "_origins"), get_analysis_result(destinations, "_destinations"), - get_analysis_result( - opt_routes_without_hazard, "_optimal_routes_without_hazard" - ), - get_analysis_result( - opt_routes_with_hazard, "_optimal_routes_with_hazard" - ), + get_analysis_result(destinations, "_optimal_routes"), get_analysis_result(_nodes_graph, "_results_nodes"), get_analysis_result(_edges_graph, "_results_edges"), ] ) + if not opt_routes_with_hazard.empty: + _analysis_result_wrapper.results_collection.append( + get_analysis_result(opt_routes_without_hazard, "_optimal_routes") + ) + if not opt_routes_without_hazard.empty: + _analysis_result_wrapper.results_collection.append( + get_analysis_result(opt_routes_with_hazard, "_optimal_routes") + ) if self.graph_file_hazard.file is not None: agg_results.to_excel( From 1ec170a62234b872e8f3181241a860715fe905e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Tue, 10 Dec 2024 16:54:08 +0100 Subject: [PATCH 05/17] chore: small refactor to reduce code duplication --- ra2ce/analysis/analysis_base.py | 23 +++++++++----- .../multi_link_origin_closest_destination.py | 30 ++++++++----------- ...ptimal_route_origin_closest_destination.py | 19 +++--------- 3 files changed, 31 insertions(+), 41 deletions(-) diff --git a/ra2ce/analysis/analysis_base.py b/ra2ce/analysis/analysis_base.py index 71389ed7a..d545ef318 100644 --- a/ra2ce/analysis/analysis_base.py +++ b/ra2ce/analysis/analysis_base.py @@ -34,6 +34,18 @@ class AnalysisBase(ABC, AnalysisProtocol): the `AnalysisProtocol`. """ + def _get_analysis_result( + self, gdf_result: GeoDataFrame, custom_name: str + ) -> AnalysisResult: + _ar = AnalysisResult( + analysis_result=gdf_result, + analysis_config=self.analysis, + output_path=self.output_path, + ) + if custom_name: + _ar.analysis_name = custom_name + return _ar + def generate_result_wrapper( self, *analysis_result: GeoDataFrame ) -> AnalysisResultWrapper: @@ -48,13 +60,8 @@ def generate_result_wrapper( AnalysisResultWrapper: Wrapping result with configuration details. """ - def get_analysis_result(gdf_result: GeoDataFrame) -> AnalysisResult: - return AnalysisResult( - analysis_result=gdf_result, - analysis_config=self.analysis, - output_path=self.output_path, - ) - return AnalysisResultWrapper( - results_collection=list(map(get_analysis_result, analysis_result)) + results_collection=[ + self._get_analysis_result(_ar, "") for _ar in analysis_result + ] ) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index 8df3d6e97..9fc1705bf 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -47,17 +47,6 @@ def __init__( self._analysis_input = analysis_input def execute(self) -> AnalysisResultWrapper: - def get_analysis_result( - gdf_result: GeoDataFrame, suffix_name: str - ) -> AnalysisResult: - _ar = AnalysisResult( - analysis_result=gdf_result, - analysis_config=self.analysis, - output_path=self.output_path, - ) - _ar.analysis_name = self.analysis.name.replace(" ", "_") + suffix_name - return _ar - _output_path = self.output_path.joinpath(self.analysis.analysis.config_value) analyzer = OriginClosestDestination(self._analysis_input) @@ -96,22 +85,27 @@ def get_analysis_result( opt_routes_without_hazard = GeoDataFrame() _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(_base_graph) + _base_name = self.analysis.name.replace(" ", "_") _analysis_result_wrapper = AnalysisResultWrapper( results_collection=[ - get_analysis_result(origins, "_origins"), - get_analysis_result(destinations, "_destinations"), - get_analysis_result(destinations, "_optimal_routes"), - get_analysis_result(_nodes_graph, "_results_nodes"), - get_analysis_result(_edges_graph, "_results_edges"), + self._get_analysis_result(origins, _base_name + "_origins"), + self._get_analysis_result(destinations, _base_name + "_destinations"), + self._get_analysis_result(destinations, _base_name + "_optimal_routes"), + self._get_analysis_result(_nodes_graph, _base_name + "_results_nodes"), + self._get_analysis_result(_edges_graph, _base_name + "_results_edges"), ] ) if not opt_routes_with_hazard.empty: _analysis_result_wrapper.results_collection.append( - get_analysis_result(opt_routes_without_hazard, "_optimal_routes") + self._get_analysis_result( + opt_routes_without_hazard, _base_name + "_optimal_routes" + ) ) if not opt_routes_without_hazard.empty: _analysis_result_wrapper.results_collection.append( - get_analysis_result(opt_routes_with_hazard, "_optimal_routes") + self._get_analysis_result( + opt_routes_with_hazard, _base_name + "_optimal_routes" + ) ) if self.graph_file_hazard.file is not None: diff --git a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py index c1429fbc3..9c525f175 100644 --- a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py +++ b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py @@ -44,17 +44,6 @@ def __init__( self._analysis_input = analysis_input def execute(self) -> AnalysisResultWrapper: - def get_analysis_result( - gdf_result: GeoDataFrame, suffix_name: str - ) -> AnalysisResult: - _ar = AnalysisResult( - analysis_result=gdf_result, - analysis_config=self.analysis, - output_path=self.output_path, - ) - _ar.analysis_name = self.analysis.name.replace(" ", "_") + suffix_name - return _ar - analyzer = OriginClosestDestination(self._analysis_input) # Get gdfs @@ -63,11 +52,11 @@ def get_analysis_result( opt_routes, destinations, ) = analyzer.optimal_route_origin_closest_destination() - + _base_name = self.analysis.name.replace(" ", "_") return AnalysisResultWrapper( results_collection=[ - get_analysis_result(base_graph, "_origins"), - get_analysis_result(destinations, "_destinations"), - get_analysis_result(opt_routes, "_optimal_routes"), + self._get_analysis_result(base_graph, _base_name + "_origins"), + self._get_analysis_result(destinations, _base_name + "_destinations"), + self._get_analysis_result(opt_routes, _base_name + "_optimal_routes"), ] ) From 6e15ec283c739504694c5830b61c1bdb63b843be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Tue, 10 Dec 2024 16:54:41 +0100 Subject: [PATCH 06/17] chore: removed unnecessary references --- ra2ce/analysis/losses/multi_link_origin_closest_destination.py | 1 - .../losses/optimal_route_origin_closest_destination.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index 9fc1705bf..3b92007df 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -7,7 +7,6 @@ AnalysisSectionLosses, ) from ra2ce.analysis.analysis_input_wrapper import AnalysisInputWrapper -from ra2ce.analysis.analysis_result.analysis_result import AnalysisResult from ra2ce.analysis.analysis_result.analysis_result_wrapper import AnalysisResultWrapper from ra2ce.analysis.losses.analysis_losses_protocol import AnalysisLossesProtocol from ra2ce.analysis.losses.origin_closest_destination import OriginClosestDestination diff --git a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py index 9c525f175..9bb60b24d 100644 --- a/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py +++ b/ra2ce/analysis/losses/optimal_route_origin_closest_destination.py @@ -1,13 +1,10 @@ from pathlib import Path -from geopandas import GeoDataFrame - from ra2ce.analysis.analysis_base import AnalysisBase from ra2ce.analysis.analysis_config_data.analysis_config_data import ( AnalysisSectionLosses, ) from ra2ce.analysis.analysis_input_wrapper import AnalysisInputWrapper -from ra2ce.analysis.analysis_result.analysis_result import AnalysisResult from ra2ce.analysis.analysis_result.analysis_result_wrapper import AnalysisResultWrapper from ra2ce.analysis.losses.analysis_losses_protocol import AnalysisLossesProtocol from ra2ce.analysis.losses.origin_closest_destination import OriginClosestDestination From 6fe2bbbf164d9de81fb734283834142928c77398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 09:40:32 +0100 Subject: [PATCH 07/17] chore: Refactored code to remove unnecessary methods from `network_utils` --- .../geodataframe_network_exporter.py | 17 +++++++---- .../exporters/multi_graph_network_exporter.py | 30 ++++++++++++------- .../exporters/network_exporter_base.py | 17 +++++------ .../exporters/network_exporter_factory.py | 2 +- ra2ce/network/networks_utils.py | 8 ++--- .../test_geodataframe_network_exporter.py | 22 +++++++++----- .../test_multi_graph_network_exporter.py | 12 ++++++-- .../exporters/test_network_exporter_base.py | 2 +- 8 files changed, 66 insertions(+), 44 deletions(-) diff --git a/ra2ce/network/exporters/geodataframe_network_exporter.py b/ra2ce/network/exporters/geodataframe_network_exporter.py index 433ceaabc..727e15b14 100644 --- a/ra2ce/network/exporters/geodataframe_network_exporter.py +++ b/ra2ce/network/exporters/geodataframe_network_exporter.py @@ -29,13 +29,18 @@ class GeoDataFrameNetworkExporter(NetworkExporterBase): def export_to_gpkg(self, output_dir: Path, export_data: gpd.GeoDataFrame) -> None: - _output_shp_path = output_dir / (self._basename + ".gpkg") + _output_gpkg_path = output_dir.joinpath(self.basename + ".gpkg") + + if _output_gpkg_path.exists(): + logging.info("Removing previous gpkg file %s.", _output_gpkg_path) + _output_gpkg_path.unlink() + export_data.to_file( - _output_shp_path, index=False - ) # , encoding='utf-8' -Removed the encoding type because this causes some shapefiles not to save. - logging.info(f"Saved {_output_shp_path.stem} in {output_dir}.") + _output_gpkg_path, index=False, driver="GPKG", encoding="utf-8" + ) + logging.info("Saved %s in %s.", _output_gpkg_path.stem, output_dir) def export_to_pickle(self, output_dir: Path, export_data: gpd.GeoDataFrame) -> None: - self.pickle_path = output_dir / (self._basename + ".feather") + self.pickle_path = output_dir.joinpath(self.basename + ".feather") export_data.to_feather(self.pickle_path, index=False) - logging.info(f"Saved {self.pickle_path.stem} in {output_dir}.") + logging.info("Saved %s in %s.", self.pickle_path.stem, output_dir) diff --git a/ra2ce/network/exporters/multi_graph_network_exporter.py b/ra2ce/network/exporters/multi_graph_network_exporter.py index 47941ff54..89120fc88 100644 --- a/ra2ce/network/exporters/multi_graph_network_exporter.py +++ b/ra2ce/network/exporters/multi_graph_network_exporter.py @@ -24,11 +24,14 @@ from pathlib import Path from typing import Optional +from ra2ce.network.exporters.geodataframe_network_exporter import ( + GeoDataFrameNetworkExporter, +) from ra2ce.network.exporters.network_exporter_base import ( MULTIGRAPH_TYPE, NetworkExporterBase, ) -from ra2ce.network.networks_utils import graph_to_gpkg +from ra2ce.network.networks_utils import get_nodes_and_edges_from_origin_graph class MultiGraphNetworkExporter(NetworkExporterBase): @@ -38,20 +41,27 @@ def export_to_gpkg(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None if not output_dir.is_dir(): output_dir.mkdir(parents=True) - # TODO: This method should be a writer itself. - graph_to_gpkg( - export_data, - output_dir / (self._basename + "_edges.gpkg"), - output_dir / (self._basename + "_nodes.gpkg"), - ) + _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(export_data) + + # Export through the single gdf exporter + _gdf_exporter = GeoDataFrameNetworkExporter(basename=self.basename) + _gdf_exporter.basename = self.basename + "_edges.gpkg" + _gdf_exporter.export(output_dir, _edges_graph) + + _gdf_exporter.basename = self.basename + "_nodes.gpkg" + _gdf_exporter.export(output_dir, _nodes_graph) + logging.info( - f"Saved {self._basename + '_edges.gpkg'} and {self._basename + '_nodes.gpkg'} in {output_dir}." + "Saved %s and %s in %s.", + self.basename + "_edges.gpkg", + self.basename + "_nodes.gpkg", + output_dir, ) def export_to_pickle(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None: - self.pickle_path = output_dir / (self._basename + ".p") + self.pickle_path = output_dir.joinpath(self.basename + ".p") with open(self.pickle_path, "wb") as f: pickle.dump(export_data, f, protocol=4) logging.info( - f"Saved {self.pickle_path.stem} in {self.pickle_path.resolve().parent}." + "Saved %s in %s.", self.pickle_path.stem, self.pickle_path.resolve().parent ) diff --git a/ra2ce/network/exporters/network_exporter_base.py b/ra2ce/network/exporters/network_exporter_base.py index ed27a93e0..34b76bfab 100644 --- a/ra2ce/network/exporters/network_exporter_base.py +++ b/ra2ce/network/exporters/network_exporter_base.py @@ -20,6 +20,7 @@ """ +from dataclasses import dataclass, field from pathlib import Path import geopandas as gpd @@ -31,15 +32,11 @@ NETWORK_TYPE = gpd.GeoDataFrame | MULTIGRAPH_TYPE +@dataclass(kw_only=True) class NetworkExporterBase(Ra2ceExporterProtocol): - _basename: str - _export_types: list[str] = ["pickle"] - pickle_path: Path - - def __init__(self, basename: str, export_types: list[str]) -> None: - self._basename = basename - self._export_types = export_types - self.pickle_path = None + basename: str + export_types: list[str] = field(default_factory=["pickle"]) + pickle_path: Path = None def export_to_gpkg(self, output_dir: Path, export_data: NETWORK_TYPE) -> None: """ @@ -70,8 +67,8 @@ def export(self, export_path: Path, export_data: NETWORK_TYPE) -> None: export_path (Path): Path to the output directory where to export the data. export_data (NETWORK_TYPE): Data that needs to be exported. """ - if "pickle" in self._export_types: + if "pickle" in self.export_types: self.export_to_pickle(export_path, export_data) - if "gpkg" in self._export_types: + if "gpkg" in self.export_types: self.export_to_gpkg(export_path, export_data) diff --git a/ra2ce/network/exporters/network_exporter_factory.py b/ra2ce/network/exporters/network_exporter_factory.py index a403daf21..eaa6be594 100644 --- a/ra2ce/network/exporters/network_exporter_factory.py +++ b/ra2ce/network/exporters/network_exporter_factory.py @@ -47,7 +47,7 @@ def export( export_types: list[str], ) -> None: _exporter_type = self.get_exporter_type(network) - self._exporter = _exporter_type(basename, export_types) + self._exporter = _exporter_type(basename=basename, export_types=export_types) self._exporter.export(output_dir, network) def get_pickle_path(self) -> Path: diff --git a/ra2ce/network/networks_utils.py b/ra2ce/network/networks_utils.py index a508bad0d..e4caccba0 100644 --- a/ra2ce/network/networks_utils.py +++ b/ra2ce/network/networks_utils.py @@ -1182,11 +1182,9 @@ def get_nodes_and_edges_from_origin_graph( return _nodes, _edges +@DeprecationWarning def graph_to_gpkg(origin_graph: nx.Graph, edge_gpkg: str, node_gpkg: str) -> None: """ - [DEPRECATED] Please use instead `AnalysisResultWrapperExporter`. - Takes in a networkx graph object and outputs shapefiles at the paths indicated by edge_gpkg and node_gpkg - Arguments: origin_graph [nx.Graph]: networkx graph object to be converted edge_gpkg [str]: output path including extension for edges geopackage @@ -1197,8 +1195,8 @@ def graph_to_gpkg(origin_graph: nx.Graph, edge_gpkg: str, node_gpkg: str) -> Non """ _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(origin_graph) - logging.info("Saving nodes as shapefile: %s".format(node_gpkg)) - logging.info("Saving edges as shapefile: %s".format(edge_gpkg)) + logging.info("Saving nodes as shapefile: %s", node_gpkg) + logging.info("Saving edges as shapefile: %s", edge_gpkg) # The encoding utf-8 might result in an empty shapefile if the wrong encoding is used. for entity in [_nodes_graph, _edges_graph]: diff --git a/tests/network/exporters/test_geodataframe_network_exporter.py b/tests/network/exporters/test_geodataframe_network_exporter.py index e568a77aa..60e90859b 100644 --- a/tests/network/exporters/test_geodataframe_network_exporter.py +++ b/tests/network/exporters/test_geodataframe_network_exporter.py @@ -1,4 +1,5 @@ import shutil +from pathlib import Path import pytest from geopandas import GeoDataFrame @@ -7,25 +8,28 @@ GeoDataFrameNetworkExporter, ) from ra2ce.network.exporters.network_exporter_base import NetworkExporterBase -from tests import test_results class TestGeodataframeNetworkExporter: def test_initialize(self): _basename = "dummy_test" - _exporter = GeoDataFrameNetworkExporter(_basename, ["pickle", "gpkg"]) + _exporter = GeoDataFrameNetworkExporter( + basename=_basename, export_types=["pickle", "gpkg"] + ) assert isinstance(_exporter, GeoDataFrameNetworkExporter) assert isinstance(_exporter, NetworkExporterBase) @pytest.mark.skip(reason="TODO: Needs to define GeoDataFrame dummydata.") - def test_export_to_gpkg(self, request: pytest.FixtureRequest): + def test_export_to_gpkg(self, test_result_param_case: Path): # 1. Define test data. - _output_dir = test_results / request.node.name + _output_dir = test_result_param_case if _output_dir.is_dir(): shutil.rmtree(_output_dir) _basename = "dummy_test" - _exporter = GeoDataFrameNetworkExporter(_basename, ["pickle", "gpkg"]) + _exporter = GeoDataFrameNetworkExporter( + basename=_basename, export_types=["pickle", "gpkg"] + ) _export_data = GeoDataFrame() @@ -37,14 +41,16 @@ def test_export_to_gpkg(self, request: pytest.FixtureRequest): assert (_output_dir / (_basename + ".gpkg")).is_file() @pytest.mark.skip(reason="TODO: Needs to define GeoDataFrame dummydata.") - def test_export_to_pickle(self, request: pytest.FixtureRequest): + def test_export_to_pickle(self, test_result_param_case: Path): # 1. Define test data. - _output_dir = test_results / request.node.name + _output_dir = test_result_param_case if _output_dir.is_dir(): shutil.rmtree(_output_dir) _basename = "dummy_test" - _exporter = GeoDataFrameNetworkExporter(_basename, ["pickle", "gpkg"]) + _exporter = GeoDataFrameNetworkExporter( + basename=_basename, export_types=["pickle", "gpkg"] + ) _export_data = GeoDataFrame() diff --git a/tests/network/exporters/test_multi_graph_network_exporter.py b/tests/network/exporters/test_multi_graph_network_exporter.py index 266c0e17f..31c807037 100644 --- a/tests/network/exporters/test_multi_graph_network_exporter.py +++ b/tests/network/exporters/test_multi_graph_network_exporter.py @@ -12,7 +12,9 @@ class TestMultigraphNetworkExporter: def test_initialize(self): - _exporter = MultiGraphNetworkExporter("_basename", ["pickle", "gpkg"]) + _exporter = MultiGraphNetworkExporter( + basename="_basename", export_types=["pickle", "gpkg"] + ) assert isinstance(_exporter, MultiGraphNetworkExporter) assert isinstance(_exporter, NetworkExporterBase) assert isinstance(_exporter, Ra2ceExporterProtocol) @@ -23,7 +25,9 @@ def test_export_to_gpkg_creates_dir(self, request: pytest.FixtureRequest): """ # 1. Define test data. _basename = "dummy_test" - _exporter = MultiGraphNetworkExporter(_basename, ["pickle", "gpkg"]) + _exporter = MultiGraphNetworkExporter( + basename=_basename, export_types=["pickle", "gpkg"] + ) _test_dir = test_results / request.node.name if _test_dir.is_dir(): shutil.rmtree(_test_dir) @@ -42,7 +46,9 @@ def test_export_to_gpkg_creates_dir(self, request: pytest.FixtureRequest): def test_export_to_pickle(self, request: pytest.FixtureRequest): # 1. Define test data. _basename = "dummy_test" - _exporter = MultiGraphNetworkExporter(_basename, ["pickle", "gpkg"]) + _exporter = MultiGraphNetworkExporter( + basename=_basename, export_types=["pickle", "gpkg"] + ) _test_dir = test_results / request.node.name if _test_dir.is_dir(): shutil.rmtree(_test_dir) diff --git a/tests/network/exporters/test_network_exporter_base.py b/tests/network/exporters/test_network_exporter_base.py index 94ca1503f..a5e1d69ef 100644 --- a/tests/network/exporters/test_network_exporter_base.py +++ b/tests/network/exporters/test_network_exporter_base.py @@ -10,7 +10,7 @@ def test_initialize(self): _exporter_base = NetworkExporterBase("a_name", []) assert isinstance(_exporter_base, NetworkExporterBase) assert isinstance(_exporter_base, Ra2ceExporterProtocol) - assert _exporter_base._export_types == [] + assert _exporter_base.export_types == [] @pytest.mark.parametrize("export_type", [("pickle"), ("gpkg")]) def test_export_data(self, export_type: str, request: pytest.FixtureRequest): From 046b6accf2a61ecd42a6e1d001712bb2661c8c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 09:41:04 +0100 Subject: [PATCH 08/17] chore: Removed unnecessary method from `network_utils` --- ra2ce/network/networks_utils.py | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/ra2ce/network/networks_utils.py b/ra2ce/network/networks_utils.py index e4caccba0..22109caec 100644 --- a/ra2ce/network/networks_utils.py +++ b/ra2ce/network/networks_utils.py @@ -1182,34 +1182,6 @@ def get_nodes_and_edges_from_origin_graph( return _nodes, _edges -@DeprecationWarning -def graph_to_gpkg(origin_graph: nx.Graph, edge_gpkg: str, node_gpkg: str) -> None: - """ - Arguments: - origin_graph [nx.Graph]: networkx graph object to be converted - edge_gpkg [str]: output path including extension for edges geopackage - node_gpkg [str]: output path including extension for nodes geopackage - - Returns: - None - """ - _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(origin_graph) - - logging.info("Saving nodes as shapefile: %s", node_gpkg) - logging.info("Saving edges as shapefile: %s", edge_gpkg) - - # The encoding utf-8 might result in an empty shapefile if the wrong encoding is used. - for entity in [_nodes_graph, _edges_graph]: - if "osmid" in entity: - # Otherwise it gives this error: cannot insert osmid, already exist - entity["osmid_original"] = entity.pop("osmid") - for _path in [node_gpkg, edge_gpkg]: - if _path.exists(): - _path.unlink() - _nodes_graph.to_file(node_gpkg, driver="GPKG", encoding="utf-8") - _edges_graph.to_file(edge_gpkg, driver="GPKG", encoding="utf-8") - - @staticmethod def get_normalized_geojson_polygon(geojson_path: Path) -> BaseGeometry: """ From c5d61fb6fa726a4aafa5538f3e169d39cfed514e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 09:48:04 +0100 Subject: [PATCH 09/17] chore: Minor corrections after refactoring --- ra2ce/network/exporters/network_exporter_base.py | 2 +- tests/network/exporters/test_network_exporter_base.py | 6 ++++-- tests/network/test_networks.py | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ra2ce/network/exporters/network_exporter_base.py b/ra2ce/network/exporters/network_exporter_base.py index 34b76bfab..bea1027c3 100644 --- a/ra2ce/network/exporters/network_exporter_base.py +++ b/ra2ce/network/exporters/network_exporter_base.py @@ -35,7 +35,7 @@ @dataclass(kw_only=True) class NetworkExporterBase(Ra2ceExporterProtocol): basename: str - export_types: list[str] = field(default_factory=["pickle"]) + export_types: list[str] = field(default_factory=lambda: ["pickle"]) pickle_path: Path = None def export_to_gpkg(self, output_dir: Path, export_data: NETWORK_TYPE) -> None: diff --git a/tests/network/exporters/test_network_exporter_base.py b/tests/network/exporters/test_network_exporter_base.py index a5e1d69ef..fc6121eb9 100644 --- a/tests/network/exporters/test_network_exporter_base.py +++ b/tests/network/exporters/test_network_exporter_base.py @@ -7,7 +7,7 @@ class TestNetworkExporterBase: def test_initialize(self): - _exporter_base = NetworkExporterBase("a_name", []) + _exporter_base = NetworkExporterBase(basename="a_name", export_types=[]) assert isinstance(_exporter_base, NetworkExporterBase) assert isinstance(_exporter_base, Ra2ceExporterProtocol) assert _exporter_base.export_types == [] @@ -18,7 +18,9 @@ def test_export_data(self, export_type: str, request: pytest.FixtureRequest): _output_dir = test_results / request.node.name # 2. Run test. - _exporter_base = NetworkExporterBase("a_name", [export_type]) + _exporter_base = NetworkExporterBase( + basename="a_name", export_types=[export_type] + ) _result = _exporter_base.export(_output_dir, None) # 3. Verify expectations. diff --git a/tests/network/test_networks.py b/tests/network/test_networks.py index 9de008892..13c40cfb7 100644 --- a/tests/network/test_networks.py +++ b/tests/network/test_networks.py @@ -105,7 +105,7 @@ def test_network_creation( # 3. Then verify expectations. def validate_file(filename: str): - _graph_file = _output_graph_dir / filename + _graph_file = _output_graph_dir.joinpath(filename) return _graph_file.is_file() and _graph_file.exists() assert isinstance(_network_controller, GraphFilesCollection) From ea9ecda9c377a1704cac949a412b351645fc8cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 10:03:27 +0100 Subject: [PATCH 10/17] test: Corrected test assertion so the error message is more clear --- tests/test_main.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 6b99a9509..ed4906528 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -228,18 +228,19 @@ def _verify_file(filepath: Path) -> bool: return filepath.exists() and filepath.is_file() # Graph files - assert all(_verify_file(_graph_dir / _f) for _f in expected_graph_files) + assert all(_verify_file(_graph_dir.joinpath(_f)) for _f in expected_graph_files) # Analysis files - assert all( - list( - chain( - *( - list( - map(lambda x: _verify_file(_analysis_dir.joinpath(k, x)), v) - ) - for k, v in expected_analysis_files.items() - ) + _not_generated_files = [] + for _subdir_name, _subdir_files in expected_analysis_files.items(): + _not_generated_files.extend( + filter( + lambda x: not _verify_file(_analysis_dir.joinpath(_subdir_name, x)), + _subdir_files, ) ) - ) + _err_mssg = ", ".join(_not_generated_files) + if any(_not_generated_files): + pytest.fail( + "The following expected files were not generated: {}".format(_err_mssg) + ) From 91a0f45a2b6d14a01a4c52c3ec41e34870396cab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 10:13:16 +0100 Subject: [PATCH 11/17] chore: Corrected export statement, now it does to `gpkg` --- ra2ce/network/exporters/multi_graph_network_exporter.py | 4 ++-- tests/test_main.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ra2ce/network/exporters/multi_graph_network_exporter.py b/ra2ce/network/exporters/multi_graph_network_exporter.py index 89120fc88..48cc98120 100644 --- a/ra2ce/network/exporters/multi_graph_network_exporter.py +++ b/ra2ce/network/exporters/multi_graph_network_exporter.py @@ -46,10 +46,10 @@ def export_to_gpkg(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None # Export through the single gdf exporter _gdf_exporter = GeoDataFrameNetworkExporter(basename=self.basename) _gdf_exporter.basename = self.basename + "_edges.gpkg" - _gdf_exporter.export(output_dir, _edges_graph) + _gdf_exporter.export_to_gpkg(output_dir, _edges_graph) _gdf_exporter.basename = self.basename + "_nodes.gpkg" - _gdf_exporter.export(output_dir, _nodes_graph) + _gdf_exporter.export_to_gpkg(output_dir, _nodes_graph) logging.info( "Saved %s and %s in %s.", diff --git a/tests/test_main.py b/tests/test_main.py index ed4906528..9ba8490c1 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -241,6 +241,4 @@ def _verify_file(filepath: Path) -> bool: ) _err_mssg = ", ".join(_not_generated_files) if any(_not_generated_files): - pytest.fail( - "The following expected files were not generated: {}".format(_err_mssg) - ) + pytest.fail(f"The following expected files were not generated: {_err_mssg}") From 97051880d12438acb8bfb17ebec50a935531f6c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 11:10:44 +0100 Subject: [PATCH 12/17] chore: Wrong basename given during `MultiGraphNetworkExporter.export_to_gpkg` --- .../network/exporters/multi_graph_network_exporter.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/ra2ce/network/exporters/multi_graph_network_exporter.py b/ra2ce/network/exporters/multi_graph_network_exporter.py index 48cc98120..73e99bce1 100644 --- a/ra2ce/network/exporters/multi_graph_network_exporter.py +++ b/ra2ce/network/exporters/multi_graph_network_exporter.py @@ -45,19 +45,12 @@ def export_to_gpkg(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None # Export through the single gdf exporter _gdf_exporter = GeoDataFrameNetworkExporter(basename=self.basename) - _gdf_exporter.basename = self.basename + "_edges.gpkg" + _gdf_exporter.basename = self.basename + "_edge" _gdf_exporter.export_to_gpkg(output_dir, _edges_graph) - _gdf_exporter.basename = self.basename + "_nodes.gpkg" + _gdf_exporter.basename = self.basename + "_nodes" _gdf_exporter.export_to_gpkg(output_dir, _nodes_graph) - logging.info( - "Saved %s and %s in %s.", - self.basename + "_edges.gpkg", - self.basename + "_nodes.gpkg", - output_dir, - ) - def export_to_pickle(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None: self.pickle_path = output_dir.joinpath(self.basename + ".p") with open(self.pickle_path, "wb") as f: From 6366588fec1ad64c0786d95a187133a55c75749f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 11:21:31 +0100 Subject: [PATCH 13/17] chore: Corrected generation of optimal routes files --- .../losses/multi_link_origin_closest_destination.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index 3b92007df..f7c47b568 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -85,15 +85,25 @@ def execute(self) -> AnalysisResultWrapper: _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(_base_graph) _base_name = self.analysis.name.replace(" ", "_") + _opt_without_hazard = self._get_analysis_result( + opt_routes_without_hazard, _base_name + "_optimal_routes_without_hazard" + ) + _opt_with_hazard = self._get_analysis_result( + opt_routes_with_hazard, _base_name + "_optimal_routes_with_hazard" + ) + _analysis_result_wrapper = AnalysisResultWrapper( results_collection=[ self._get_analysis_result(origins, _base_name + "_origins"), self._get_analysis_result(destinations, _base_name + "_destinations"), - self._get_analysis_result(destinations, _base_name + "_optimal_routes"), self._get_analysis_result(_nodes_graph, _base_name + "_results_nodes"), self._get_analysis_result(_edges_graph, _base_name + "_results_edges"), + _opt_without_hazard, + _opt_with_hazard, ] ) + + # Legacy code, previously only done to export to CSV. if not opt_routes_with_hazard.empty: _analysis_result_wrapper.results_collection.append( self._get_analysis_result( From c87e5fef8d988cc92e49283a47a042e55ddf2745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 11:34:19 +0100 Subject: [PATCH 14/17] chore: corrected wrong suffix name `_node` instead of `_nodes` --- ra2ce/network/exporters/multi_graph_network_exporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ra2ce/network/exporters/multi_graph_network_exporter.py b/ra2ce/network/exporters/multi_graph_network_exporter.py index 73e99bce1..af7f1506c 100644 --- a/ra2ce/network/exporters/multi_graph_network_exporter.py +++ b/ra2ce/network/exporters/multi_graph_network_exporter.py @@ -45,7 +45,7 @@ def export_to_gpkg(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None # Export through the single gdf exporter _gdf_exporter = GeoDataFrameNetworkExporter(basename=self.basename) - _gdf_exporter.basename = self.basename + "_edge" + _gdf_exporter.basename = self.basename + "_edges" _gdf_exporter.export_to_gpkg(output_dir, _edges_graph) _gdf_exporter.basename = self.basename + "_nodes" From f6a626a33a20e58e6f52b9e6f11ff704b80d72a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 11:57:02 +0100 Subject: [PATCH 15/17] chore: Reordered appending of analysis result as it was done earlier --- .../losses/multi_link_origin_closest_destination.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index f7c47b568..718cc8b4a 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -104,17 +104,14 @@ def execute(self) -> AnalysisResultWrapper: ) # Legacy code, previously only done to export to CSV. - if not opt_routes_with_hazard.empty: + _opt_routes_name = _base_name + "_optimal_routes" + if not opt_routes_without_hazard.empty: _analysis_result_wrapper.results_collection.append( - self._get_analysis_result( - opt_routes_without_hazard, _base_name + "_optimal_routes" - ) + self._get_analysis_result(opt_routes_with_hazard, _opt_routes_name) ) - if not opt_routes_without_hazard.empty: + if not opt_routes_with_hazard.empty: _analysis_result_wrapper.results_collection.append( - self._get_analysis_result( - opt_routes_with_hazard, _base_name + "_optimal_routes" - ) + self._get_analysis_result(opt_routes_without_hazard, _opt_routes_name) ) if self.graph_file_hazard.file is not None: From b709e5d1f8d7b5a171c978d72963b48f4ffde140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 11:58:08 +0100 Subject: [PATCH 16/17] chore: Processed review remarks --- .../multi_link_origin_closest_destination.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py index 718cc8b4a..bf41d5f09 100644 --- a/ra2ce/analysis/losses/multi_link_origin_closest_destination.py +++ b/ra2ce/analysis/losses/multi_link_origin_closest_destination.py @@ -85,21 +85,19 @@ def execute(self) -> AnalysisResultWrapper: _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(_base_graph) _base_name = self.analysis.name.replace(" ", "_") - _opt_without_hazard = self._get_analysis_result( - opt_routes_without_hazard, _base_name + "_optimal_routes_without_hazard" - ) - _opt_with_hazard = self._get_analysis_result( - opt_routes_with_hazard, _base_name + "_optimal_routes_with_hazard" - ) - _analysis_result_wrapper = AnalysisResultWrapper( results_collection=[ self._get_analysis_result(origins, _base_name + "_origins"), self._get_analysis_result(destinations, _base_name + "_destinations"), self._get_analysis_result(_nodes_graph, _base_name + "_results_nodes"), self._get_analysis_result(_edges_graph, _base_name + "_results_edges"), - _opt_without_hazard, - _opt_with_hazard, + self._get_analysis_result( + opt_routes_without_hazard, + _base_name + "_optimal_routes_without_hazard", + ), + self._get_analysis_result( + opt_routes_with_hazard, _base_name + "_optimal_routes_with_hazard" + ), ] ) From 4c06436e4acf629181454cf324597b4fee595ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carles=20S=2E=20Soriano=20P=C3=A9rez?= Date: Wed, 11 Dec 2024 13:28:42 +0100 Subject: [PATCH 17/17] chore: Corrected logic that was preventing us from exporting the index in the geodataframes --- .../exporters/multi_graph_network_exporter.py | 17 +++++++++++------ ra2ce/network/networks_utils.py | 3 ++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/ra2ce/network/exporters/multi_graph_network_exporter.py b/ra2ce/network/exporters/multi_graph_network_exporter.py index af7f1506c..b3bf4527e 100644 --- a/ra2ce/network/exporters/multi_graph_network_exporter.py +++ b/ra2ce/network/exporters/multi_graph_network_exporter.py @@ -24,6 +24,8 @@ from pathlib import Path from typing import Optional +from geopandas import GeoDataFrame + from ra2ce.network.exporters.geodataframe_network_exporter import ( GeoDataFrameNetworkExporter, ) @@ -43,13 +45,16 @@ def export_to_gpkg(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None _nodes_graph, _edges_graph = get_nodes_and_edges_from_origin_graph(export_data) - # Export through the single gdf exporter - _gdf_exporter = GeoDataFrameNetworkExporter(basename=self.basename) - _gdf_exporter.basename = self.basename + "_edges" - _gdf_exporter.export_to_gpkg(output_dir, _edges_graph) + def export_gdf(gdf_data: GeoDataFrame, suffix: str): + """ + Different from `GeoDataFrameNetworkExporter` at `index=True`. + """ + _export_file = output_dir.joinpath(self.basename + suffix + ".gpkg") + gdf_data.to_file(_export_file, index=True, driver="GPKG", encoding="utf-8") + logging.info("Saved %s in %s.", _export_file.stem, output_dir) - _gdf_exporter.basename = self.basename + "_nodes" - _gdf_exporter.export_to_gpkg(output_dir, _nodes_graph) + export_gdf(_edges_graph, "_edges") + export_gdf(_nodes_graph, "_nodes") def export_to_pickle(self, output_dir: Path, export_data: MULTIGRAPH_TYPE) -> None: self.pickle_path = output_dir.joinpath(self.basename + ".p") diff --git a/ra2ce/network/networks_utils.py b/ra2ce/network/networks_utils.py index 22109caec..ef66d97de 100644 --- a/ra2ce/network/networks_utils.py +++ b/ra2ce/network/networks_utils.py @@ -1157,7 +1157,8 @@ def get_nodes_and_edges_from_origin_graph( resulting tuple of formatted `gpd.GeoDataFrame` for nodes and edges. """ # now only multidigraphs and graphs are used - if isinstance(origin_graph, nx.Graph): + if type(origin_graph) == nx.Graph: + # isinstance / issubclass will not work as nx.MultiGraph would return True origin_graph = nx.MultiGraph(origin_graph) # The nodes should have a geometry attribute (perhaps on top of the x and y attributes)