From 3301d59f442b1cd46b22337e48149b75a2787ed9 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:29:48 +0200 Subject: [PATCH] Fix histogram counts disagreeing with interval labels on bin boundaries (#546) --- crates/chart/src/render.rs | 8 +++++--- crates/chart/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/chart/src/render.rs b/crates/chart/src/render.rs index 63d30d982..0bb628ee1 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -1201,10 +1201,12 @@ fn histogram(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx) { bins = 1; } let width = if bins == 1 && span.partial_cmp(&0.0) != Some(std::cmp::Ordering::Greater) { 1.0 } else { span / bins as f64 }; + // Upper edge of each bin, exactly as shown in the labels below. Bins are right-inclusive (the first is closed on both sides), so a value + // equal to an edge belongs to the bin that ends there. + let edges: Vec = (0..bins).map(|b| if b + 1 == bins { hi.max(lo + b as f64 * width) } else { lo + (b + 1) as f64 * width }).collect(); let mut counts = vec![0usize; bins]; for v in &vals { - let b = if width > 0.0 { ((v - lo) / width).floor() } else { 0.0 }; - let b = if b.is_finite() && b > 0.0 { (b as usize).min(bins - 1) } else { 0 }; + let b = edges.partition_point(|e| e < v).min(bins - 1); if let Some(c) = counts.get_mut(b) { *c += 1; } @@ -1213,7 +1215,7 @@ fn histogram(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx) { let labels: Vec = (0..bins) .map(|b| { let a = lo + b as f64 * width; - let e = if b + 1 == bins { hi.max(a) } else { a + width }; + let e = edges.get(b).copied().unwrap_or(a); let open = if b == 0 { '[' } else { '(' }; format!("{open}{}, {}]", f.format(a, Some(width / 10.0)), f.format(e, Some(width / 10.0))) }) diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index a94267713..b38bf174d 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -877,6 +877,28 @@ fn treemap_and_histogram_shapes() { assert_eq!(bins, 10); } +#[test] +fn histogram_boundary_values_match_interval_labels() { + let heights = |vals: &[f64]| -> Vec { + let d = ChartData { categories: vec![], series: vec![series("h", vals, [10, 20, 30, 255], ChartKind::Histogram)] }; + let prims = render(&chart(ChartKind::Histogram), &d, 600.0, 400.0, &ApproxMeasure); + prims.iter().filter_map(|p| if let Prim::Rect { h, fill: Some([10, 20, 30, 255]), .. } = p { Some(*h) } else { None }).collect() + }; + // Labels are "[0, 7.5]" and "(7.5, 15.0]": 7.5 belongs to the first bar, so the counts are 2 and 1. + let h = heights(&[0.0, 7.5, 15.0]); + assert_eq!(h.len(), 2); + assert!((h[0] / h[1] - 2.0).abs() < 0.01, "{h:?}"); + let h = heights(&[-15.0, -7.5, 0.0]); + assert!((h[0] / h[1] - 2.0).abs() < 0.01, "{h:?}"); + // A value off the boundary still goes right: [0, 7.5] holds 1, (7.5, 15.0] holds 2. + let h = heights(&[0.0, 7.75, 15.0]); + assert!((h[1] / h[0] - 2.0).abs() < 0.01, "{h:?}"); + // Five values, three bins [0,5] (5,10] (10,15]: counts 3, 1, 1. + let h = heights(&[0.0, 5.0, 5.0, 10.0, 15.0]); + assert_eq!(h.len(), 3); + assert!((h[0] / h[1] - 3.0).abs() < 0.01 && (h[1] - h[2]).abs() < 0.01, "{h:?}"); +} + #[test] fn union_series_references() { let mut wb = Workbook::new();