From 4139a040bb993c4d54c83890e9da0e1ec9e7ce7e Mon Sep 17 00:00:00 2001
From: Daniel Himmelstein <daniel.himmelstein@gmail.com>
Date: Sat, 16 Mar 2024 20:34:56 -0400
Subject: [PATCH] _bearings_distribution: clarity, comments, tests

---
 osmnx/bearing.py    | 29 ++++++++++++++++++-----------
 osmnx/plot.py       |  5 ++---
 tests/test_osmnx.py | 18 ++++++++++++++++++
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/osmnx/bearing.py b/osmnx/bearing.py
index 3b625d98e..d6dfeac0a 100644
--- a/osmnx/bearing.py
+++ b/osmnx/bearing.py
@@ -272,17 +272,24 @@ def _bearings_distribution(
         Counts of bearings per bin and the bins' centers in degrees.
         Both arrays are of length `num_bins`.
     """
-    n = num_bins * 2
-    bins = np.arange(n + 1) * 360 / n
+    # Split bins in half to prevent bin-edge effects around common values.
+    # Bins will be merged in pairs after the histogram is computed.
+    # The last bin edge is the same as the first (i.e., 0 degrees = 360 degrees).
+    num_split_bins = num_bins * 2
+    split_bin_edges = np.arange(num_split_bins + 1) * 360 / num_split_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
-    count = np.roll(count, 1)
-    bin_counts = count[::2] + count[1::2]
-
-    # because we merged the bins, their centers are now only every other one
-    bin_centers = bin_edges[range(0, len(bin_edges) - 1, 2)]
+    split_bin_counts, split_bin_edges = np.histogram(
+        bearings,
+        bins=split_bin_edges,
+        weights=weights,
+    )
+
+    # Move last bin to front, so eg 0.01 degrees and 359.99 degrees will be
+    # binned together. Then combine counts from pairs of split bins.
+    split_bin_counts = np.roll(split_bin_counts, 1)
+    bin_counts = split_bin_counts[::2] + split_bin_counts[1::2]
+
+    # Every other edge of the split bins is the center of a merged bin.
+    bin_centers = split_bin_edges[range(0, num_split_bins - 1, 2)]
     return bin_counts, bin_centers
diff --git a/osmnx/plot.py b/osmnx/plot.py
index dd85e2b66..772783189 100644
--- a/osmnx/plot.py
+++ b/osmnx/plot.py
@@ -748,7 +748,7 @@ def plot_orientation(  # noqa: PLR0913
             "zorder": 3,
         }
 
-    # get the bearings' distribution's bin counts and edges
+    # get the bearing distribution's bin counts and center values in degrees
     bin_counts, bin_centers = bearing._bearings_distribution(
         G,
         num_bins,
@@ -756,8 +756,7 @@ def plot_orientation(  # noqa: PLR0913
         weight=weight,
     )
 
-    # positions: where to center each bar. ignore the last bin edge, because
-    # it's the same as the first (i.e., 0 degrees = 360 degrees)
+    # positions: where to center each bar
     positions = np.radians(bin_centers)
 
     # width: make bars fill the circumference without gaps or overlaps
diff --git a/tests/test_osmnx.py b/tests/test_osmnx.py
index e3441e303..976405d3e 100644
--- a/tests/test_osmnx.py
+++ b/tests/test_osmnx.py
@@ -165,6 +165,24 @@ def test_bearings() -> None:
     assert list(bearings) == [0.0, 180.0]  # north and south
     assert list(weights) == [2.0, 2.0]
 
+    # test _bearings_distribution split bin implementation
+    bin_counts, bin_centers = ox.bearing._bearings_distribution(
+        G,
+        num_bins=1,
+        min_length=0,
+        weight=None,
+    )
+    assert list(bin_counts) == [1.0]
+    assert list(bin_centers) == [0.0]
+    bin_counts, bin_centers = ox.bearing._bearings_distribution(
+        G,
+        num_bins=2,
+        min_length=0,
+        weight=None,
+    )
+    assert list(bin_counts) == [1.0, 0.0]
+    assert list(bin_centers) == [0.0, 180.0]
+
 
 def test_osm_xml() -> None:
     """Test working with .osm XML data."""