From ea3ef8faf7813489bf6332255cef203e9a054b6b Mon Sep 17 00:00:00 2001 From: webdevtodayjason <115739170+webdevtodayjason@users.noreply.github.com> Date: Fri, 16 Oct 2026 12:25:48 -0500 Subject: [PATCH] Charts: honor box and whisker quartile methods Keep exclusive and inclusive quartiles per series when drawing and saving XLSX, and expose the setting through chart.set. Fixes #1453 --- crates/chart/src/render.rs | 44 +++++++++++++++++-- crates/chart/src/tests.rs | 32 ++++++++++++++ crates/engine/src/cmd/forecast.rs | 1 + crates/engine/src/cmd/insert.rs | 17 ++++++- crates/engine/src/pivot.rs | 1 + crates/engine/src/sample.rs | 1 + crates/engine/src/tests.rs | 27 ++++++++++++ crates/model/src/features.rs | 11 +++++ crates/xlsx/src/chart.rs | 1 + crates/xlsx/src/chartex.rs | 42 ++++++++++++++++-- .../src/tests/literal_category_roundtrip.rs | 1 + crates/xlsx/src/tests/roundtrip.rs | 1 + crates/xlsx/src/tests/smooth_roundtrip.rs | 1 + crates/xlsx/src/tests/trendline_roundtrip.rs | 1 + docs/chart-parity.md | 5 ++- 15 files changed, 175 insertions(+), 11 deletions(-) diff --git a/crates/chart/src/render.rs b/crates/chart/src/render.rs index 037b9099e..af30df572 100644 --- a/crates/chart/src/render.rs +++ b/crates/chart/src/render.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use std::f32::consts::{PI, TAU}; -use gridcraft_model::{Chart, ChartKind, LegendPos, LineDash, Theme, TrendlineKind}; +use gridcraft_model::{Chart, ChartKind, LegendPos, LineDash, QuartileMethod, Theme, TrendlineKind}; use crate::axis::{Fmt, Scale, clean, fixed_scale, nice_scale}; use crate::resolve::{MAX_POINTS, MAX_SERIES, MAX_TRENDLINES, series_color}; @@ -1561,12 +1561,16 @@ fn radar(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx) { // ---------------------------------------------------------------- box & whisker -fn quantile(sorted: &[f64], q: f64) -> f64 { +fn quantile(sorted: &[f64], q: f64, method: QuartileMethod) -> f64 { let n = sorted.len(); if n == 0 { return 0.0; } - let pos = q * (n - 1) as f64; + // Clamp exclusive ranks for short samples so one or two points remain drawable. + let pos = match method { + QuartileMethod::Inclusive => q * (n - 1) as f64, + QuartileMethod::Exclusive => (q * (n as f64 + 1.0) - 1.0).clamp(0.0, (n - 1) as f64), + }; let i = pos.floor() as usize; let frac = pos - i as f64; let a = sorted.get(i).copied().unwrap_or(0.0); @@ -1593,7 +1597,8 @@ fn box_whisker(ctx: &mut Ctx, chart: &Chart, data: &ChartData, area: Bx) { continue; } v.sort_by(f64::total_cmp); - let (q1, med, q3) = (quantile(&v, 0.25), quantile(&v, 0.5), quantile(&v, 0.75)); + let method = chart.series.get(i).map(|s| s.quartiles).unwrap_or_default(); + let (q1, med, q3) = (quantile(&v, 0.25, method), quantile(&v, 0.5, method), quantile(&v, 0.75, method)); let iqr = q3 - q1; let (flo, fhi) = (q1 - 1.5 * iqr, q3 + 1.5 * iqr); let wlo = v.iter().copied().find(|x| *x >= flo).unwrap_or(q1); @@ -1769,3 +1774,34 @@ mod smooth_tests { assert!(out.iter().all(|p| (0.0..=300.0).contains(&p[0]) && (10.0..=110.0).contains(&p[1]))); } } + +#[cfg(test)] +mod quartile_tests { + use super::*; + + #[test] + fn box_whisker_default_quartiles_are_exclusive() { + let values = [1.0, 2.0, 3.0, 4.0, 5.0, 6.0]; + assert_eq!(quantile(&values, 0.25, QuartileMethod::default()), 1.75); + assert_eq!(quantile(&values, 0.5, QuartileMethod::default()), 3.5); + assert_eq!(quantile(&values, 0.75, QuartileMethod::default()), 5.25); + } + + #[test] + fn quartile_methods_and_short_samples() { + for (values, exclusive, inclusive) in [ + (vec![1., 2., 3.], (1., 3.), (1.5, 2.5)), + (vec![1., 2., 3., 4.], (1.25, 3.75), (1.75, 3.25)), + (vec![1., 2., 3., 4., 5.], (1.5, 4.5), (2., 4.)), + (vec![1., 2., 3., 4., 5., 6., 7.], (2., 6.), (2.5, 5.5)), + (vec![1., 1., 2., 3., 5., 8.], (1., 5.75), (1.25, 4.5)), + (vec![1., 2.], (1., 2.), (1.25, 1.75)), + (vec![1.], (1., 1.), (1., 1.)), + (vec![], (0., 0.), (0., 0.)), + ] { + for (method, expected) in [(QuartileMethod::Exclusive, exclusive), (QuartileMethod::Inclusive, inclusive)] { + assert_eq!((quantile(&values, 0.25, method), quantile(&values, 0.75, method)), expected); + } + } + } +} diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index 01f1aac49..07729c6c7 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -227,6 +227,7 @@ fn resolve_from_workbook() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }, @@ -238,6 +239,7 @@ fn resolve_from_workbook() { color: Some(Color::rgb(1, 2, 3)), secondary: true, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }, @@ -249,6 +251,7 @@ fn resolve_from_workbook() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }, @@ -287,6 +290,7 @@ fn resolve_from_workbook() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }, @@ -298,6 +302,7 @@ fn resolve_from_workbook() { color: None, secondary: true, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }, @@ -314,6 +319,7 @@ fn resolve_from_workbook() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }]; @@ -331,6 +337,7 @@ fn resolve_from_workbook() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }]; @@ -1001,6 +1008,7 @@ fn pie_points_follow_workbook_theme() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }]; @@ -1057,3 +1065,27 @@ fn area_charts_draw_source_value_labels_only_when_enabled() { assert!(first_label > last_poly, "{kind:?}: label drawn under a later area"); } } + +#[test] +fn box_whisker_draws_each_series_quartile_method() { + use gridcraft_model::QuartileMethod; + let mut c = chart(ChartKind::BoxWhisker); + c.legend = LegendPos::None; + c.series = vec![Series::default(), Series { quartiles: QuartileMethod::Inclusive, ..Default::default() }]; + let color = [12, 34, 56, 255]; + let d = ChartData { + series: vec![series("A", &[1., 2., 3., 4., 5., 6.], color, c.kind), series("B", &[1., 2., 3., 4., 5., 6.], color, c.kind)], + ..Default::default() + }; + let prims = render(&c, &d, 640., 400., &ApproxMeasure); + let heights: Vec = prims + .iter() + .filter_map(|p| match p { + Prim::Rect { h, fill, .. } if *fill == Some(color) => Some(*h), + _ => None, + }) + .collect(); + assert_eq!(heights.len(), 2); + // The boxes span 3.5 and 2.5 data units on the same value axis. + assert!((heights[0] / heights[1] - 1.4).abs() < 0.001); +} diff --git a/crates/engine/src/cmd/forecast.rs b/crates/engine/src/cmd/forecast.rs index d25b367ba..139d6fc46 100644 --- a/crates/engine/src/cmd/forecast.rs +++ b/crates/engine/src/cmd/forecast.rs @@ -411,6 +411,7 @@ fn forecast_sheet(s: &mut Session, p: &Json) -> Result { color: None, secondary: false, kind: (opts.column_chart && c >= 3).then_some(ChartKind::Line), + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }) diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index d878e4469..b320fd7fe 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -90,7 +90,7 @@ pub fn specs() -> Vec { "Chart Properties", ["Chart Design"], None, - "{chart?: id, type?, title?, legend?: none|bottom|top|left|right, dataLabels?, gridlines?, style?, xTitle?, yTitle?, at?, width?, height?, dx?, dy?}", + "{chart?: id, type?, title?, legend?: none|bottom|top|left|right, dataLabels?, gridlines?, quartiles?: exclusive|inclusive, style?, xTitle?, yTitle?, at?, width?, height?, dx?, dy?}", has_doc, chart_set ), @@ -603,6 +603,7 @@ pub fn series_from_range(sh: &Sheet, r: RangeRef, by_rows: bool) -> Vec color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }); @@ -618,6 +619,7 @@ pub fn series_from_range(sh: &Sheet, r: RangeRef, by_rows: bool) -> Vec color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }); @@ -788,12 +790,25 @@ fn find_chart(s: &Session, p: &Json) -> Result<(usize, usize)> { fn chart_set(s: &mut Session, p: &Json) -> Result { let (si, ci) = find_chart(s, p)?; let at = cell_param_on(s, p, "at", si)?; + let quartiles = p + .get("quartiles") + .map(|v| match v.as_str() { + Some("exclusive") => Ok(gridcraft_model::QuartileMethod::Exclusive), + Some("inclusive") => Ok(gridcraft_model::QuartileMethod::Inclusive), + _ => Err(bad("quartiles", "expected exclusive or inclusive")), + }) + .transpose()?; edit(s, |cx| { let sh = cx.sheet_mut(si)?; let Some(c) = sh.charts.get_mut(ci) else { return Ok(Json::Null) }; if let Some(t) = str_param(p, "type") { c.kind = chart_kind(t, str_param(p, "subtype").unwrap_or("")); } + if let Some(method) = quartiles { + for series in &mut c.series { + series.quartiles = method; + } + } if let Some(t) = p.get("title") { c.title = t.as_str().map(str::to_string); } diff --git a/crates/engine/src/pivot.rs b/crates/engine/src/pivot.rs index 6146bd4f9..e8a33fd50 100644 --- a/crates/engine/src/pivot.rs +++ b/crates/engine/src/pivot.rs @@ -1567,6 +1567,7 @@ pub fn chart_series(sheet: &str, origin: CellRef, out: &Output) -> Vec { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }) diff --git a/crates/engine/src/sample.rs b/crates/engine/src/sample.rs index 4d4822445..129fe2171 100644 --- a/crates/engine/src/sample.rs +++ b/crates/engine/src/sample.rs @@ -108,6 +108,7 @@ fn sales(s: &mut Session) -> bool { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }]; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..a650b8271 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3282,3 +3282,30 @@ fn paste_special_validation_moves_relative_references() { assert!(!ok(&mut s, "C3", "5"), "the pasted rule checks C3, not A1"); assert!(ok(&mut s, "C3", "20")); } + +#[test] +fn box_whisker_quartile_methods_survive_xlsx() { + use gridcraft_model::QuartileMethod; + let mut s = s(); + chart_data(&mut s); + s.execute("insert.chart", json!({"range": "A1:C5", "type": "boxWhisker"})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().charts[0].series[0].quartiles, QuartileMethod::Exclusive); + s.execute("chart.set", json!({"quartiles": "inclusive"})).unwrap(); + assert!(s.doc().unwrap().wb.active().unwrap().charts[0].series.iter().all(|s| s.quartiles == QuartileMethod::Inclusive)); + for invalid in [json!("median"), json!(null), json!(1)] { + assert!(s.execute("chart.set", json!({"quartiles": invalid, "title": "Changed"})).is_err()); + } + assert_ne!(s.doc().unwrap().wb.active().unwrap().charts[0].title.as_deref(), Some("Changed")); + // Imported series can differ; changing a chart's setting applies to all its series. + let mut wb = (*s.doc().unwrap().wb).clone(); + let sh = wb.sheet_mut(0).unwrap(); + assert!(sh.charts[0].series.len() > 1); + sh.charts[0].series[1].quartiles = QuartileMethod::Exclusive; + let bytes = gridcraft_xlsx::write_xlsx(&wb).unwrap(); + let (back, _) = gridcraft_xlsx::read_xlsx(&bytes).unwrap(); + let series = &back.active().unwrap().charts[0].series; + assert_eq!(series[0].quartiles, QuartileMethod::Inclusive); + assert_eq!(series[1].quartiles, QuartileMethod::Exclusive); + s.execute("chart.set", json!({"quartiles": "exclusive"})).unwrap(); + assert!(s.doc().unwrap().wb.active().unwrap().charts[0].series.iter().all(|s| s.quartiles == QuartileMethod::Exclusive)); +} diff --git a/crates/model/src/features.rs b/crates/model/src/features.rs index b51f7bc98..01c98425e 100644 --- a/crates/model/src/features.rs +++ b/crates/model/src/features.rs @@ -406,6 +406,14 @@ pub enum LegendPos { Right, } +/// Quartile calculation for a box & whisker series. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub enum QuartileMethod { + #[default] + Exclusive, + Inclusive, +} + #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct Series { /// Formula text for the series name (`Sheet1!$B$1`) or a literal. @@ -421,6 +429,9 @@ pub struct Series { /// Draw line segments as a smoothed (spline) curve. #[serde(default)] pub smooth: bool, + /// Box & whisker quartiles; Excel defaults to exclusive. + #[serde(default)] + pub quartiles: QuartileMethod, /// Trendlines fitted to this series (Excel allows several per series). #[serde(default)] pub trendlines: Vec, diff --git a/crates/xlsx/src/chart.rs b/crates/xlsx/src/chart.rs index dee33c83d..1b893a3d2 100644 --- a/crates/xlsx/src/chart.rs +++ b/crates/xlsx/src/chart.rs @@ -106,6 +106,7 @@ fn read_series(s: &El) -> Series { color, secondary: false, kind: None, + quartiles: Default::default(), smooth: bool_val(s, "smooth"), trendlines: s.kids("trendline").filter_map(read_trendline).collect(), } diff --git a/crates/xlsx/src/chartex.rs b/crates/xlsx/src/chartex.rs index 2ab8c221b..e32fc63d0 100644 --- a/crates/xlsx/src/chartex.rs +++ b/crates/xlsx/src/chartex.rs @@ -6,7 +6,7 @@ use std::fmt::Write as _; use gridcraft_core::RangeRef; use gridcraft_formula::{Expr, SheetSel}; -use gridcraft_model::{Chart, ChartKind, LegendPos, Series, Workbook}; +use gridcraft_model::{Chart, ChartKind, LegendPos, QuartileMethod, Series, Workbook}; use crate::drawing::{dml_color, srgb}; use crate::xml::{El, esc, esc_attr}; @@ -99,6 +99,10 @@ pub fn read_chartex(root: &El) -> Option { let (cat, val) = data.iter().find(|(i, _, _)| i == id).map(|(_, c, v)| (c.clone(), v.clone())).unwrap_or_default(); data_labels |= s.path(&["dataLabels", "visibility"]).is_some_and(|v| v.flag("value", false)); series.push(Series { + quartiles: match s.path(&["layoutPr", "statistics"]).and_then(|el| el.attr("quartileMethod")) { + Some("inclusive") => QuartileMethod::Inclusive, + _ => QuartileMethod::Exclusive, + }, name: s.child("tx").and_then(text), categories: cat, values: val.unwrap_or_default(), @@ -309,9 +313,16 @@ pub fn write_chartex(wb: &Workbook, si: usize, ch: &Chart, names: &mut Vec"); match ch.kind { ChartKind::Histogram => s.push_str(""), - ChartKind::BoxWhisker => s.push_str( - "", - ), + ChartKind::BoxWhisker => { + let method = match ser.quartiles { + QuartileMethod::Exclusive => "exclusive", + QuartileMethod::Inclusive => "inclusive", + }; + let _ = write!( + s, + "" + ); + } ChartKind::Treemap => s.push_str(""), _ => {} } @@ -364,3 +375,26 @@ pub fn write_chartex(wb: &Workbook, si: usize, ch: &Chart, names: &mut Vec", QuartileMethod::Exclusive), + (r#""#, QuartileMethod::Exclusive), + (r#""#, QuartileMethod::Exclusive), + (r#""#, QuartileMethod::Inclusive), + ] { + let xml = format!( + r#"{statistics}"# + ); + let root = crate::xml::parse(xml.as_bytes()).unwrap(); + let chart = read_chartex(&root).unwrap(); + assert_eq!(chart.series[0].quartiles, expected); + } + } +} diff --git a/crates/xlsx/src/tests/literal_category_roundtrip.rs b/crates/xlsx/src/tests/literal_category_roundtrip.rs index c818de221..e9f6a4461 100644 --- a/crates/xlsx/src/tests/literal_category_roundtrip.rs +++ b/crates/xlsx/src/tests/literal_category_roundtrip.rs @@ -28,6 +28,7 @@ fn literal_category_with_separators_survives_save() { color: None, secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }) diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..bedf79e5b 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -380,6 +380,7 @@ fn sample() -> Workbook { color: Some(Color::Rgb(0xFF0000)), secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }], diff --git a/crates/xlsx/src/tests/smooth_roundtrip.rs b/crates/xlsx/src/tests/smooth_roundtrip.rs index 5a4f68fb2..0b4ebcef8 100644 --- a/crates/xlsx/src/tests/smooth_roundtrip.rs +++ b/crates/xlsx/src/tests/smooth_roundtrip.rs @@ -17,6 +17,7 @@ fn smooth_line_setting_round_trips() { color: Some(Color::Rgb(0x4472C4)), secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines: Vec::new(), }; diff --git a/crates/xlsx/src/tests/trendline_roundtrip.rs b/crates/xlsx/src/tests/trendline_roundtrip.rs index d954c9bc6..02948a6c7 100644 --- a/crates/xlsx/src/tests/trendline_roundtrip.rs +++ b/crates/xlsx/src/tests/trendline_roundtrip.rs @@ -47,6 +47,7 @@ fn chart(id: u32, kind: ChartKind, trendlines: Vec) -> Chart { color: Some(Color::Rgb(0x4472C4)), secondary: false, kind: None, + quartiles: Default::default(), smooth: false, trendlines, }], diff --git a/docs/chart-parity.md b/docs/chart-parity.md index 9058b0935..9bb87339a 100644 --- a/docs/chart-parity.md +++ b/docs/chart-parity.md @@ -1,6 +1,6 @@ # Chart parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** trivial (chart sheets are kept on save, still not editable (#727); previously minor: trendlines: model, XLSX round-trip, drawing, Add Chart Element) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** minor (box & whisker quartile methods are read, written and drawn per series; chart sheets are kept on save, still not editable (#727); previously minor: trendlines: model, XLSX round-trip, drawing, Add Chart Element) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Excel's chart types and chart elements against GridCraft. Part of [target-app-parity.md](target-app-parity.md). **Breadth ~50%, ready ~30%, 40–60 h** (estimated). @@ -25,7 +25,7 @@ open PRs #158, #161, and issue #190. | Radar | radar, with markers, filled | 1 of 3 | | | Treemap, Sunburst | 1 each | 2 of 2 | lose their kind in XLSX (PR #161) | | Histogram, Pareto | 2 | 1 of 2 | no Pareto | -| Box & Whisker | 1 | 1 of 1 | loses its kind in XLSX (PR #161) | +| Box & Whisker | 1 | 1 of 1 | exclusive (default) and inclusive quartiles per series; XLSX round-trip; `chart.set {quartiles}` changes all series | | Waterfall, Funnel | 2 | 2 of 2 | lose their kind in XLSX (PR #161) | | Filled map | 1 | 0 | needs geographic boundary data (licensing decision) | | Combo | custom combinations, secondary axis | yes (line on secondary axis) | | @@ -80,6 +80,7 @@ back. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | Box & whisker quartiles default to exclusive, honor each series on draw and XLSX round-trip; `chart.set {quartiles}` changes the method (#1453) | | 2026-10-12 | trivial | chart sheets are kept on save, still not editable (#727) | | 2026-10-11 | minor | Trendlines: all six types modelled, round-tripped through `c:trendline`, drawn with forecast and equation/R² labels; `chart.addElement` `trendline` | | 2026-10-11 | minor | PivotCharts (`insert.pivotChart`) |