Merge pull request #1147 from dhimmel/dsh-issue-1143

_bearings_distribution: defer weighting to np.histogram

CHANGELOG.md

@@ -23,6 +23,7 @@ Read the v2 [migration guide](https://github.com/gboeing/osmnx/issues/1123)
 - make utils_geo.bbox_from_point function return a tuple of floats for consistency with rest of package (#1113)
 - change add_node_elevations_google default batch_size to 512 to match Google's limit (#1115)
 - allow analysis of MultiDiGraph directional edge bearings and orientation (#1139)
+- perform efficient and precise weighting when calculating an edge bearing distribution (#1147)
 - fix bug in \_downloader.\_save_to_cache function usage (#1107)
 - fix bug in handling requests ConnectionError when querying Overpass status endpoint (#1113)
 - fix minor bugs throughout to address inconsistencies revealed by type enforcement (#1107 #1114)

osmnx/bearing.py

@@ -176,7 +176,7 @@ def _extract_edge_bearings(
     G: nx.MultiGraph | nx.MultiDiGraph,
     min_length: float,
     weight: str | None,
-) -> npt.NDArray[np.float64]:
+) -> tuple[npt.NDArray[np.float64], npt.NDArray[np.float64]]:
     """
     Extract graph's edge bearings.
 
@@ -195,44 +195,45 @@ def _extract_edge_bearings(
         Ignore edges with `length` attributes less than `min_length`. Useful
         to ignore the noise of many very short edges.
     weight
-        If not None, weight edges' bearings by this (non-null) edge attribute.
-        For example, if "length" is provided, this will return 1 bearing
-        observation per meter per street (which could result in a very large
-        `bearings` array).
+        If None, return equal weight for each bearing. Otherwise,
+        weight edges' bearings by this (non-null) edge attribute.
+        For example, if "length" is provided, this will weight each bearing
+        observation by the meter length of each street.
 
     Returns
     -------
-    bearings
-        The edge bearings of `Gu`.
+    bearings, weights
+        The edge bearings of `G` and their corresponding weights.
     """
     if projection.is_projected(G.graph["crs"]):  # pragma: no cover
         msg = "Graph must be unprojected to analyze edge bearings."
         raise ValueError(msg)
     bearings = []
+    weights = []
     for u, v, data in G.edges(data=True):
         # ignore self-loops and any edges below min_length
         if u != v and data["length"] >= min_length:
-            if weight:
-                # weight edges' bearings by some edge attribute value
-                bearings.extend([data["bearing"]] * int(data[weight]))
-            else:
-                # don't weight bearings, just take one value per edge
-                bearings.append(data["bearing"])
+            bearings.append(data["bearing"])
+            weights.append(data[weight] if weight is not None else 1.0)
 
     # drop any nulls
     bearings_array = np.array(bearings)
-    bearings_array = bearings_array[~np.isnan(bearings_array)]
+    weights_array = np.array(weights)
+    keep_idx = ~np.isnan(bearings_array)
+    bearings_array = bearings_array[keep_idx]
+    weights_array = weights_array[keep_idx]
     if nx.is_directed(G):
         msg = (
             "`G` is a MultiDiGraph, so edge bearings will be directional (one per "
             "edge). If you want bidirectional edge bearings (two reciprocal bearings "
             "per edge), pass a MultiGraph instead. Use `convert.to_undirected`."
         )
         warn(msg, category=UserWarning, stacklevel=2)
-        return bearings_array
-    # for undirected graphs, add reverse bearings and return
-    bearings_array_r = (bearings_array - 180) % 360
-    return np.concatenate([bearings_array, bearings_array_r])
+        return bearings_array, weights_array
+    # for undirected graphs, add reverse bearings
+    bearings_array = np.concatenate([bearings_array, (bearings_array - 180) % 360])
+    weights_array = np.concatenate([weights_array, weights_array])
+    return bearings_array, weights_array
 
 
 def _bearings_distribution(
@@ -260,10 +261,10 @@ def _bearings_distribution(
         Ignore edges with `length` attributes less than `min_length`. Useful
         to ignore the noise of many very short edges.
     weight
-        If not None, weight edges' bearings by this (non-null) edge attribute.
-        For example, if "length" is provided, this will return 1 bearing
-        observation per meter per street (which could result in a very large
-        `bearings` array).
+        If None, apply equal weight for each bearing. Otherwise,
+        weight edges' bearings by this (non-null) edge attribute.
+        For example, if "length" is provided, this will weight each bearing
+        observation by the meter length of each street.
 
     Returns
     -------
@@ -273,8 +274,8 @@ def _bearings_distribution(
     n = num_bins * 2
     bins = np.arange(n + 1) * 360 / n
 
-    bearings = _extract_edge_bearings(G, min_length, weight)
-    count, bin_edges = np.histogram(bearings, bins=bins)
+    bearings, weights = _extract_edge_bearings(G, min_length, weight)
+    count, bin_edges = np.histogram(bearings, bins=bins, weights=weights)
 
     # move last bin to front, so eg 0.01 degrees and 359.99 degrees will be
     # binned together

tests/test_osmnx.py

@@ -150,18 +150,20 @@ def test_bearings() -> None:
     G = nx.MultiDiGraph(crs="epsg:4326")
     G.add_node("point_1", x=0.0, y=0.0)
     G.add_node("point_2", x=0.0, y=1.0)  # latitude increases northward
-    G.add_edge("point_1", "point_2")
+    G.add_edge("point_1", "point_2", weight=2.0)
     G = ox.distance.add_edge_lengths(G)
     G = ox.add_edge_bearings(G)
     with pytest.warns(UserWarning, match="edge bearings will be directional"):
-        bearings = ox.bearing._extract_edge_bearings(G, min_length=0, weight=None)
+        bearings, weights = ox.bearing._extract_edge_bearings(G, min_length=0, weight=None)
     assert list(bearings) == [0.0]  # north
-    bearings = ox.bearing._extract_edge_bearings(
+    assert list(weights) == [1.0]
+    bearings, weights = ox.bearing._extract_edge_bearings(
         ox.convert.to_undirected(G),
         min_length=0,
-        weight=None,
+        weight="weight",
     )
     assert list(bearings) == [0.0, 180.0]  # north and south
+    assert list(weights) == [2.0, 2.0]
 
 
 def test_osm_xml() -> None: