From 8c3becbaf479f052d9bc26243fd633946df3715a Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sat, 9 May 2026 11:44:44 -0700 Subject: [PATCH] Improve formatting of axis tick labels. --- rust/doc/src/spv/graphs.md | 9 + rust/pspp/src/spv/read/graph.rs | 86 +++++++-- rust/pspp/src/spv/read/graph/format.rs | 63 +++--- .../pspp/src/spv/testdata/graph1.expected.svg | 24 +-- .../pspp/src/spv/testdata/graph2.expected.svg | 180 +++++++++--------- .../pspp/src/spv/testdata/graph3.expected.svg | 76 ++++---- .../pspp/src/spv/testdata/graph4.expected.svg | 62 +++--- 7 files changed, 285 insertions(+), 215 deletions(-) diff --git a/rust/doc/src/spv/graphs.md b/rust/doc/src/spv/graphs.md index 40f193a4c9..6544d9b99b 100644 --- a/rust/doc/src/spv/graphs.md +++ b/rust/doc/src/spv/graphs.md @@ -904,3 +904,12 @@ stripes like \ => 2 stripes like / => 8 stripes like | => 3 stripes like - => 9 + + +Formatting of axis labels comes from the variable specified on an +element placed on that axis. If there is more than one such variable, +it appears that the variable with the name alphabetically earlier is +used. Whether a given variable has a specified format is not +considered. The details are muddled; for example, it appears that if +one of the variables lacks a specified format entirely, then none of +their formats is used. diff --git a/rust/pspp/src/spv/read/graph.rs b/rust/pspp/src/spv/read/graph.rs index a8603e57ef..6f0b9d66e3 100644 --- a/rust/pspp/src/spv/read/graph.rs +++ b/rust/pspp/src/spv/read/graph.rs @@ -400,6 +400,7 @@ impl Visualization { fn pass<'a>( &'a self, graph: &'a Graph, + locale: &'static Locale<'static>, domains: &BTreeMap<&str, Domain>, series: &BTreeMap<&str, Series>, bin_widths: &mut BTreeMap, @@ -426,10 +427,15 @@ impl Visualization { } else { None }; + let mut formatter = None; for element in &elements { - if let Some(variable) = &element.variables[axis] - && !element.ignore_minmax[axis] - { + let Some(variable) = &element.variables[axis] else { + continue; + }; + if formatter.is_none() { + formatter = Some(variable.formatter.clone()); + } + if !element.ignore_minmax[axis] { let domain = domain.get_or_insert_with(|| { if variable.categorical { Domain::new_categorical() @@ -462,7 +468,6 @@ impl Visualization { } } } - let mut domain = domain.unwrap_or_default(); if let Domain::Interval { min, max, exact } = &mut domain && (!*exact || min.is_none() || max.is_none()) @@ -501,6 +506,7 @@ impl Visualization { .and_then(|dimension| dimension.axis.first()); axis_domains[axis] = AxisDomain { domain, + formatter: formatter.unwrap_or_else(|| Formatter::default_for_locale(locale)), label: axis2.and_then(|axis| axis.label.as_ref()), major_ticks: axis2.and_then(|axis| axis.major_ticks.as_ref()), }; @@ -625,6 +631,7 @@ impl Visualization { document.append(self.decode_graph( graph_frame, graph, + locale, &domains, &series, &style_cycles, @@ -639,6 +646,7 @@ impl Visualization { &self, graph_frame: &Frame, graph: &Graph, + locale: &'static Locale<'static>, domains: &BTreeMap<&str, Domain>, series: &BTreeMap<&str, Series>, style_cycles: &HashMap<&str, &StyleCycle>, @@ -646,10 +654,12 @@ impl Visualization { ) -> Result { let mut graph_svg = Group::new().with_id(&graph.id); let mut bin_widths = BTreeMap::new(); - let mut axis_domains = EnumMap::::default(); + let mut axis_domains = + EnumMap::::from_fn(|_| AxisDomain::default_for_locale(locale)); let mut patterns = Patterns::new(); self.pass( &graph, + locale, &domains, &series, &mut bin_widths, @@ -658,6 +668,7 @@ impl Visualization { )?; let elements = self.pass( &graph, + locale, &domains, &series, &mut bin_widths, @@ -1648,14 +1659,23 @@ impl Variable for DerivedVariable { } } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] struct AxisDomain<'a> { domain: Domain, + formatter: Formatter, label: Option<&'a Label>, major_ticks: Option<&'a MajorTicks>, } impl<'a> AxisDomain<'a> { + fn default_for_locale(locale: &'static Locale<'static>) -> Self { + Self { + domain: Domain::default(), + formatter: Formatter::default_for_locale(locale), + label: None, + major_ticks: None, + } + } fn ticks(&self) -> Vec<(f64, String)> { let mut ticks = Vec::new(); match &self.domain { @@ -1664,9 +1684,23 @@ impl<'a> AxisDomain<'a> { max: Some(max), exact: _, } => { - for x in Scaling::for_range(*min..*max).ticks() { - if let Some(frac) = self.domain.lookup(&Datum::Number(Some(x))) { - ticks.push((frac, format!("{x:.1}"))); + let scaling = Scaling::for_range(*min..*max); + let formatter = match &self.formatter { + Formatter::Number(number_formatter) + if number_formatter.maximum_fraction_digits.is_none() => + { + Cow::Owned(Formatter::Number( + number_formatter.clone().with_maximum_fraction_digits(Some( + scaling.maximum_fraction_digits(), + )), + )) + } + _ => Cow::Borrowed(&self.formatter), + }; + for x in scaling.ticks() { + let datum = Datum::Number(Some(x)); + if let Some(frac) = self.domain.lookup(&datum) { + ticks.push((frac, formatter.display(&datum).to_string())); } } } @@ -1957,6 +1991,7 @@ impl RawDomain for IntervalDomain { } } } else { + dbg!(variable.reference.as_str()); todo!() } } @@ -4434,6 +4469,27 @@ impl Scaling { } } + pub fn maximum_fraction_digits(&self) -> usize { + // Base-10 logarithm of the interval: + // 10 => 1 + // 1 => 0 + // .1 => -1 + // .2 => -1 + // .5 => -1 + // .01 => -2 + // .02 => -2 + // .05 => -2 + let log10 = (self.interval * 1.01).log10().floor() as isize; + + // 10 => 0 + // 1 => 0 + // .1 => 1 + // .01 => 2 + let result = (-log10).max(0) as usize; + dbg!(self, result); + result + } + pub fn for_range(range: impl Borrow>) -> Scaling { let range = range.borrow(); let r = range.end - range.start; @@ -5022,11 +5078,12 @@ mod tests { #[test] fn scaling() { - for (range, (base, interval, n_ticks)) in [ - (5.5..9.2, (5.0, 1.0, 6)), - (0.0..10.0, (0.0, 2.0, 6)), - (55.0..92.0, (50.0, 10.0, 6)), - (0.0..100.0, (0.0, 20.0, 6)), + for (range, (base, interval, n_ticks), maximum_fraction_digits) in [ + (5.5..9.2, (5.0, 1.0, 6), 0), + (0.55..0.92, (0.5, 0.1, 6), 1), + (0.0..10.0, (0.0, 2.0, 6), 0), + (55.0..92.0, (50.0, 10.0, 6), 0), + (0.0..100.0, (0.0, 20.0, 6), 0), ] { let expected = Scaling::new(base, interval, n_ticks); let scaling = Scaling::for_range(&range); @@ -5036,6 +5093,7 @@ mod tests { assert_eq!(scaling.first(), Some(scaling.base)); assert!(scaling.base <= range.start); assert!(scaling.last().unwrap() >= range.end); + assert_eq!(scaling.maximum_fraction_digits(), maximum_fraction_digits); } } diff --git a/rust/pspp/src/spv/read/graph/format.rs b/rust/pspp/src/spv/read/graph/format.rs index 88bed0865a..02f8490fbe 100644 --- a/rust/pspp/src/spv/read/graph/format.rs +++ b/rust/pspp/src/spv/read/graph/format.rs @@ -21,7 +21,7 @@ use crate::{ }; #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] -enum Negatives { +pub enum Negatives { /// Show negative values with a minus sign. #[default] Minus, @@ -49,7 +49,7 @@ impl Negatives { } #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] -enum NegativeLocation { +pub enum NegativeLocation { /// Show negatives inside the prefix or suffix, e.g. `$-123` or `(55)%`. #[default] Inside, @@ -58,7 +58,7 @@ enum NegativeLocation { } #[derive(Copy, Clone, Debug, Default, PartialEq, Deserialize)] -enum Scientific { +pub enum Scientific { #[default] Auto, OnlyForSmall(f64), @@ -96,13 +96,14 @@ enum NumberExpression { #[derive(Clone, Debug)] pub struct NumberFormatter { - locale: &'static Locale<'static>, - minimum_fraction_digits: Option, - maximum_fraction_digits: Option, - minimum_integer_digits: Option, - maximum_integer_digits: Option, - negatives: Negatives, - negative_location: NegativeLocation, + pub locale: &'static Locale<'static>, + + pub minimum_fraction_digits: Option, + pub maximum_fraction_digits: Option, + pub minimum_integer_digits: Option, + pub maximum_integer_digits: Option, + pub negatives: Negatives, + pub negative_location: NegativeLocation, /// A number to divide the value by before formatting. /// @@ -111,22 +112,22 @@ pub struct NumberFormatter { /// - `scaling_factor` of 1000 with a `suffix` of `thousands`. /// /// - `scaling_factor` of 1e-6 with a `suffix` of `μ`. - scaling_factor: f64, + pub scaling_factor: f64, /// When to use scientific notation. - scientific: Scientific, + pub scientific: Scientific, /// Decimal point (commonly `.` or `,`). - decimal: char, + pub decimal: char, /// Grouping character (commonly `,` or `.`), only if grouping is enabled. - group: Option, + pub group: Option, /// For indicating errors (commonly `*`). - error: char, + pub error: char, /// Prefix (such as `$`). - prefix: String, + pub prefix: String, /// Suffix (such as `%`). - suffix: String, + pub suffix: String, } #[derive(Clone, Debug)] @@ -187,7 +188,7 @@ impl Default for NumberFormatter { } impl NumberFormatter { - fn default_for_locale(locale: &'static Locale<'static>) -> Self { + pub fn default_for_locale(locale: &'static Locale<'static>) -> Self { Self { minimum_fraction_digits: None, maximum_fraction_digits: None, @@ -206,7 +207,7 @@ impl NumberFormatter { } } - fn new(format: &Format, locale: &'static Locale<'static>) -> Self { + pub fn new(format: &Format, locale: &'static Locale<'static>) -> Self { Self { maximum_fraction_digits: format.maximum_fraction_digits, minimum_fraction_digits: format.minimum_fraction_digits, @@ -252,55 +253,56 @@ impl NumberFormatter { } } - fn with_scientific(self, scientific: Scientific) -> Self { + pub fn with_scientific(self, scientific: Scientific) -> Self { Self { scientific, ..self } } - fn with_minimum_fraction_digits(self, minimum_fraction_digits: Option) -> Self { + pub fn with_minimum_fraction_digits(self, minimum_fraction_digits: Option) -> Self { Self { minimum_fraction_digits, ..self } } - fn with_maximum_fraction_digits(self, maximum_fraction_digits: Option) -> Self { + pub fn with_maximum_fraction_digits(self, maximum_fraction_digits: Option) -> Self { Self { maximum_fraction_digits, ..self } } - fn with_minimum_integer_digits(self, minimum_integer_digits: Option) -> Self { + pub fn with_minimum_integer_digits(self, minimum_integer_digits: Option) -> Self { Self { minimum_integer_digits, ..self } } - fn with_maximum_integer_digits(self, maximum_integer_digits: Option) -> Self { + pub fn with_maximum_integer_digits(self, maximum_integer_digits: Option) -> Self { Self { maximum_integer_digits, ..self } } - fn with_negatives(self, negatives: Negatives) -> Self { + pub fn with_negatives(self, negatives: Negatives) -> Self { Self { negatives, ..self } } - fn with_negative_location(self, negative_location: NegativeLocation) -> Self { + pub fn with_negative_location(self, negative_location: NegativeLocation) -> Self { Self { negative_location, ..self } } - fn with_prefix(self, prefix: impl Into) -> Self { + pub fn with_prefix(self, prefix: impl Into) -> Self { Self { prefix: prefix.into(), ..self } } - fn with_suffix(self, suffix: impl Into) -> Self { + pub fn with_suffix(self, suffix: impl Into) -> Self { Self { suffix: suffix.into(), ..self } } - fn display<'a>(&'a self, value: &'a Datum) -> DisplayNumberFormatter<'a> { + + pub fn display<'a>(&'a self, value: &'a Datum) -> impl Display { DisplayNumberFormatter { format: self, value, @@ -337,6 +339,7 @@ impl NumberFormatter { fn format_decimal(&self, f: &mut std::fmt::Formatter, number: f64) -> std::fmt::Result { let body = number.abs().to_small_string(); + dbg!(number, &body); let pn = ParsedNumber::from_decimal(body).unwrap(); let body; let pn = if let Some(maximum_fraction_digits) = self.maximum_fraction_digits @@ -439,7 +442,7 @@ pub struct DateFormatter { } impl DateFormatter { - fn new(format: &Format, locale: &'static Locale<'static>) -> Self { + pub fn new(format: &Format, locale: &'static Locale<'static>) -> Self { let mut show_date = format.show_era.unwrap_or(false) || format.show_year.unwrap_or(false) || format.show_quarter.unwrap_or(false) diff --git a/rust/pspp/src/spv/testdata/graph1.expected.svg b/rust/pspp/src/spv/testdata/graph1.expected.svg index 6494e8b6b4..dae4035671 100644 --- a/rust/pspp/src/spv/testdata/graph1.expected.svg +++ b/rust/pspp/src/spv/testdata/graph1.expected.svg @@ -9,27 +9,27 @@ ROC Curve -0.0 +0 -0.2 +0,2 -0.4 +0,4 -0.6 +0,6 -0.8 +0,8 -1.0 +1 X Axis @@ -39,27 +39,27 @@ X Axis -0.0 +0 -0.2 +0,2 -0.4 +0,4 -0.6 +0,6 -0.8 +0,8 -1.0 +1 Y Axis diff --git a/rust/pspp/src/spv/testdata/graph2.expected.svg b/rust/pspp/src/spv/testdata/graph2.expected.svg index 24dad4c95c..a1d6d25b29 100644 --- a/rust/pspp/src/spv/testdata/graph2.expected.svg +++ b/rust/pspp/src/spv/testdata/graph2.expected.svg @@ -3,148 +3,148 @@ Title - - - -10.0 + + + +10 - - -15.0 + + +15 - + V2 - + - - -0.0 + + +0 - - -10.0 + + +10 - - -20.0 + + +20 - - -30.0 + + +30 - - -40.0 + + +40 - - -50.0 + + +50 - - -60.0 + + +60 - + - + - - - -0.6 + + + +0.575 - - -0.6 + + +0.58 - - -0.6 + + +0.585 - + V1 - + - - -0.0 + + +0 - - -10.0 + + +10 - - -20.0 + + +20 - - -30.0 + + +30 - - -40.0 + + +40 - - -50.0 + + +50 - + - + - - - -30.0 + + + +30 - - -35.0 + + +35 - - -40.0 + + +40 - + V3 - + - - -0.0 + + +0 - - -10.0 + + +10 - - -20.0 + + +20 - - -30.0 + + +30 - + - + diff --git a/rust/pspp/src/spv/testdata/graph3.expected.svg b/rust/pspp/src/spv/testdata/graph3.expected.svg index 47a964691a..866c0e6572 100644 --- a/rust/pspp/src/spv/testdata/graph3.expected.svg +++ b/rust/pspp/src/spv/testdata/graph3.expected.svg @@ -1,70 +1,70 @@ - - - + + + Value0 - - + + Value1 - + V0 - + - - -0.0 + + +.0% - - -10.0 + + +10.0% - - -20.0 + + +20.0% - - -30.0 + + +30.0% Percent - + - - -0.0 + + +.0% - - -10.0 + + +10.0% - - -20.0 + + +20.0% - - -30.0 + + +30.0% Percent - + - - + + - + - - + + diff --git a/rust/pspp/src/spv/testdata/graph4.expected.svg b/rust/pspp/src/spv/testdata/graph4.expected.svg index 2d277fd075..b8c23486d6 100644 --- a/rust/pspp/src/spv/testdata/graph4.expected.svg +++ b/rust/pspp/src/spv/testdata/graph4.expected.svg @@ -1,52 +1,52 @@ - - - -0.0 + + + +0 - - -10.0 + + +10 - - -20.0 + + +20 - - -30.0 + + +30 - - -40.0 + + +40 - - -50.0 + + +50 - + - - -0.0 + + +0 - + - - -0.0 + + +0 - + - + - + - + -- 2.47.3