Merge pull request #89 from NERDSITU/88-joss-review-questions-on-00-m…

…wepy

Improve plotting behaviour and labelling

docs/conf.py

@@ -24,7 +24,7 @@
 project = "superblockify"
 copyright = "2023-2024, superblockify developers"
 author = "superblockify developers"
-release = "1.0.0rc9"
+release = "1.0.0rc10"
 
 # -- General configuration ---------------------------------------------------
 # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration

docs/usage.myst

@@ -45,7 +45,7 @@ There are several options to configure `superblockify`, see the [API Reference](
 
 Common ones are, for example, to log at the "debug" level:
 
-```{code-cell} ipython3
+```python
 sb.config.set_log_level("DEBUG")
 ```
 

superblockify/_version.py

@@ -1,3 +1,3 @@
 """superblockify package version."""
 
-__version__ = "1.0.0rc9"
+__version__ = "1.0.0rc10"

superblockify/metrics/distances.py

@@ -155,8 +155,8 @@ def calculate_path_distance_matrix(
                 [graph.nodes[node]["x"] for node in node_order],
                 [graph.nodes[node]["y"] for node in node_order],
             ),
-            coord_title="Coordinates of nodes",
-            labels=("x", "y"),
+            coord_title="Projected coordinates of nodes",
+            labels=("x (m)", "y (m)"),
             distance_unit=unit_symbol,
         )
 
@@ -230,7 +230,7 @@ def calculate_euclidean_distance_matrix_projected(
             dist_title="Distribution of Euclidean distances",
             coords=(x_coord, y_coord),
             coord_title="Scatter plot of projected coordinates",
-            labels=("x", "y"),
+            labels=("x (m)", "y (m)"),
         )
 
     return dist_matrix
@@ -325,8 +325,8 @@ def calculate_euclidean_distance_matrix_haversine(
             dist_matrix,
             dist_title="Distribution of Euclidean distances",
             coords=(node1_lon, node1_lat),
-            coord_title="Scatter plot of lat/lon",
-            labels=("Longitude", "Latitude"),
+            coord_title="Scatter plot of unprojected coordinates",
+            labels=("Longitude [°]", "Latitude [°]"),
         )
 
     return dist_matrix
@@ -447,8 +447,8 @@ def calculate_partitioning_distance_matrix(
                 [partitioner.graph.nodes[node]["x"] for node in node_order],
                 [partitioner.graph.nodes[node]["y"] for node in node_order],
             ),
-            coord_title="Coordinates of nodes",
-            labels=("x", "y"),
+            coord_title="Projected coordinates of nodes",
+            labels=("x (m)", "y (m)"),
             distance_unit=unit_symbol,
         )
 

superblockify/metrics/metric.py

@@ -20,7 +20,6 @@
 )
 from .plot import (
     plot_distance_matrices,
-    plot_distance_matrices_pairwise_relative_difference,
     plot_component_wise_travel_increase,
     plot_relative_difference,
     plot_relative_increase_on_graph,
@@ -325,7 +324,11 @@ def calculate_all(
         self.calculate_all_measure_sums()
 
         write_relative_increase_to_edges(
-            partitioner.graph, self.distance_matrix, self.node_list, "N", "S"
+            partitioner.graph,
+            self.distance_matrix,
+            self.node_list,
+            "N",
+            "S",
         )
         add_ltn_means(
             partitioner.get_ltns(),
@@ -373,35 +376,27 @@ def make_all_plots(self, partitioner):
         partitioner : BasePartitioner
             The partitioner object to calculate the metrics for
         """
-
-        fig, _ = plot_distance_matrices(
-            self, name=f"{partitioner.name} - {partitioner.__class__.__name__}"
-        )
-        save_plot(
-            partitioner.results_dir,
-            fig,
-            f"{partitioner.name}_distance_matrices.{Config.PLOT_SUFFIX}",
-        )
-        fig.show()
-        fig, _ = plot_distance_matrices_pairwise_relative_difference(
-            self, name=f"{partitioner.name} - {partitioner.__class__.__name__}"
-        )
-        save_plot(
-            partitioner.results_dir,
-            fig,
-            f"{partitioner.name}_distance_matrices_"
-            f"pairwise_relative_difference.{Config.PLOT_SUFFIX}",
-        )
-        fig.show()
-        fig, _ = plot_relative_difference(
-            self, "N", "S", title=f"{partitioner.name} - {self.__class__.__name__}"
-        )
-        save_plot(
-            partitioner.results_dir,
-            fig,
-            f"{partitioner.name}_relative_difference_SN.{Config.PLOT_SUFFIX}",
-        )
-        fig.show()
+        if logger.getEffectiveLevel() <= 10:
+            fig, _ = plot_distance_matrices(
+                self, name=f"{partitioner.name} - {partitioner.__class__.__name__}"
+            )
+            save_plot(
+                partitioner.results_dir,
+                fig,
+                f"{partitioner.name}_distance_matrices.{Config.PLOT_SUFFIX}",
+            )
+            fig, _ = plot_relative_difference(
+                self,
+                "N",
+                "S",
+                title=f"{partitioner.name} - Relative difference in "
+                f"{self.__class__.__name__}",
+            )
+            save_plot(
+                partitioner.results_dir,
+                fig,
+                f"{partitioner.name}_relative_difference_SN." f"{Config.PLOT_SUFFIX}",
+            )
         fig, _ = plot_component_wise_travel_increase(
             partitioner,
             self.distance_matrix,

superblockify/metrics/plot.py

@@ -38,16 +38,18 @@ def plot_distance_distributions(
     # Plot distribution of distances
     axe[0].hist(dist_matrix.flatten(), bins=100)
     axe[0].set_title(dist_title)
-    axe[0].set_xlabel(f"Distance [{distance_unit}]")
+    axe[0].set_xlabel(
+        "Travel time (s)" if distance_unit == "s" else f"Distance ({distance_unit})"
+    )
     axe[0].set_ylabel("Count")
     # Plot scatter plot of lat/lon, aspect ratio should be 1:1
     axe[1].set_aspect("equal")
     axe[1].scatter(coords[0], coords[1], alpha=0.5, s=1)
+    axe[1].ticklabel_format(axis="both", style="sci", scilimits=(0, 0))
     axe[1].set_title(coord_title)
     axe[1].set_xlabel(labels[0])
     axe[1].set_ylabel(labels[1])
     plt.tight_layout()
-    plt.show()
 
 
 def plot_distance_matrices(metric, name=None):
@@ -329,7 +331,7 @@ def plot_component_wise_travel_increase(
     )
     # Label the colorbar, vertical alignment, latex math mode
     cbar.ax.set_ylabel(
-        rf"Part. increase ({unit}) "
+        rf"Travel distance increase in superblock ({unit}) "
         rf"$d_{{{measure1}}} (i, j)$ / $d_{{{measure2}}} (i, j)$",
         rotation=270,
         labelpad=20,
@@ -393,7 +395,7 @@ def plot_relative_difference(metric, key_i, key_j, title=None):
     )
     axe.set_title(
         f"{title} ({metric.unit_symbol()}) | "
-        f"$\\frac{{d_{{{key_j}}}(i, j)}}{{d_{{{key_i}}}(i, j)}}$"
+        f"$\\frac{{d_{{{key_i}}}(i, j)}}{{d_{{{key_j}}}(i, j)}}$"
     )
     axe.set_xlabel("Node $j$")
     axe.set_ylabel("Node $i$")
@@ -443,7 +445,7 @@ def plot_relative_increase_on_graph(graph, unit_symbol, **pg_kwargs):
     # Label the colorbar, vertical alignment, latex math mode
     cbar.ax.set_ylabel(
         f"Travel distance increase ({unit_symbol}) "
-        "(all to all demand) $d_{S}(i, j)$ / $d_{N}(i, j)$",
+        "(all to all demand) $d_{N}(i, j)$ / $d_{S}(i, j)$",
         rotation=270,
         labelpad=20,
         fontsize=15,

superblockify/partitioning/approaches/bearing.py

@@ -94,7 +94,6 @@ def partition_graph(
             save_plot(
                 self.results_dir, fig, f"{self.name}_peakfinding.{Config.PLOT_SUFFIX}"
             )
-            plt.show()
 
         # Make boundaries
         self.__make_boundaries()
@@ -106,7 +105,6 @@ def partition_graph(
                 fig,
                 f"{self.name}_interval_splitting.{Config.PLOT_SUFFIX}",
             )
-            plt.show()
 
         # Write grouping attribute to graph
         self.attribute_label = "bearing_group"

superblockify/partitioning/base.py

@@ -20,7 +20,6 @@
 from .checks import is_valid_partitioning
 from .plot import (
     plot_partition_graph,
-    plot_subgraph_component_size,
     plot_component_rank_size,
     plot_component_graph,
     plot_speed_un_restricted,
@@ -246,37 +245,26 @@ def run(
                     fig,
                     f"{self.name}_partition_graph.{Config.PLOT_SUFFIX}",
                 )
-                plt.show()
-            fig, _ = plot_subgraph_component_size(self, "length")
-            save_plot(
-                self.results_dir,
-                fig,
-                f"{self.name}_subgraph_component_size.{Config.PLOT_SUFFIX}",
-            )
-            plt.show()
             fig, _ = plot_component_rank_size(self, "length")
             save_plot(
                 self.results_dir,
                 fig,
                 f"{self.name}_component_rank_size.{Config.PLOT_SUFFIX}",
             )
-            plt.show()
             if self.components:
                 fig, _ = plot_component_graph(self)
                 save_plot(
                     self.results_dir,
                     fig,
                     f"{self.name}_component_graph.{Config.PLOT_SUFFIX}",
                 )
-                plt.show()
             if replace_max_speeds:
                 fig, _ = plot_speed_un_restricted(self.graph, self.sparsified)
                 save_plot(
                     self.results_dir,
                     fig,
                     f"{self.name}_speed_un_restricted.{Config.PLOT_SUFFIX}",
                 )
-                fig.show()
 
         if calculate_metrics:
             self.calculate_metrics(
@@ -285,6 +273,9 @@ def run(
             )
             calculate_component_metrics(self.get_ltns())
 
+        if make_plots:
+            plt.show()
+
     @abstractmethod
     def partition_graph(self, make_plots=False, **kwargs):  # pragma: no cover
         """Partition the graph.

superblockify/partitioning/checks.py

@@ -1,13 +1,12 @@
 """Checks for the partitioning module."""
 
 from itertools import chain
-from sys import modules
 
 from networkx import is_weakly_connected
 from numpy import argwhere, fill_diagonal
 from osmnx import plot_graph
 
-from ..config import logger, Config
+from ..config import logger
 from ..plot import plot_by_attribute
 from ..utils import has_pairwise_overlap
 
@@ -125,7 +124,6 @@ def components_are_connected(partitioning):
                 edge_attr_types="numerical",
                 edge_cmap="hsv",
                 edge_minmax_val=(0, 1),
-                show="pytest" not in modules or not Config.HIDE_PLOTS,
             )
             # Reset edge attribute 'highlight'
             for edge in component["subgraph"].edges:
@@ -202,7 +200,6 @@ def nodes_and_edges_are_contained_in_exactly_one_subgraph(partitioning):
                 for node in partitioning.graph.nodes
             ],
             bgcolor="none",
-            show="pytest" not in modules or not Config.HIDE_PLOTS,
         )
         is_valid = False
 

superblockify/partitioning/plot.py

@@ -211,10 +211,10 @@ def plot_component_rank_size(partitioner, measure):
         color="k",
         zorder=2,
     )
-    axe.set_xlabel("Component rank", fontsize=12)
-    axe.set_ylabel(f"Component size ({measure} [m])", fontsize=12)
+    axe.set_xlabel("Superblock rank", fontsize=12)
+    axe.set_ylabel(f"Superblock size ({measure} (m))", fontsize=12)
     axe.set_title(
-        f"Component size rank for {partitioner.name} with attribute "
+        f"Superblock size rank for {partitioner.name} with attribute "
         f"`{partitioner.attribute_label}`",
         fontsize=13,
     )
@@ -363,7 +363,6 @@ def plot_speed_un_restricted(
         edge_cmap=cmap,
         edge_minmax_val=minmax_val,
         ax=axes[0],
-        show=False,
     )
 
     # Plot restricted max speed limits on right side
@@ -373,7 +372,6 @@ def plot_speed_un_restricted(
         edge_cmap=cmap,
         edge_minmax_val=minmax_val,
         ax=axes[1],
-        show=False,
     )
 
     # Set titles

superblockify/plot.py

@@ -216,6 +216,7 @@ def plot_by_attribute(
             node_color=node_color if node_color else (0, 0, 0, 0),
             edge_linewidth=edge_linewidth,
             bgcolor=(0, 0, 0, 0),
+            show=False,
             **pg_kwargs,
         )
     # If only node_attr is set
@@ -228,6 +229,7 @@ def plot_by_attribute(
             node_color=n_c,
             edge_linewidth=edge_linewidth,
             bgcolor=(0, 0, 0, 0),
+            show=False,
             **pg_kwargs,
         )
     # If both edge_attr and node_attr are set
@@ -238,6 +240,7 @@ def plot_by_attribute(
         node_color=n_c,
         edge_linewidth=edge_linewidth,
         bgcolor=(0, 0, 0, 0),
+        show=False,
         **pg_kwargs,
     )
 
@@ -501,7 +504,7 @@ def plot_component_size(
         Value of the partition for each component
     size_measure_label : str
         Label of the size measure (e.g. "Number of edges", "Number of nodes",
-        "Length [m]")
+        "Length (m)")
     ignore : list, optional
         List of values to ignore, plot in gray. If None, no values are ignored.
     title : str, optional

superblockify/population/tessellation.py

@@ -151,7 +151,6 @@ def get_edge_cells(graph, limit=None, segment=25, show_plot=False):
         axe.set_axis_off()
         axe.set_title("Edge tessellation")
         fig.tight_layout()
-        plt.show()
 
     logger.info(
         "Tessellated %d edge cells in %s.",