From 432694b6dc816336a4b8cf98796d77b296ef5e1e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 11:54:42 +0100 Subject: [PATCH 01/19] Centralize aesthetic metadata in unified DefaultAesthetics structure Restructure GeomAesthetics to prepare for centralized default aesthetic values. Each geom now defines all aesthetic metadata (required, optional, delayed) in a single defaults field, enabling future control over visual appearance defaults. Key changes: - Rename GeomAesthetics to DefaultAesthetics with unified defaults field - Add Required, Null, Delayed variants to DefaultAestheticValue enum - Derive supported/required lists via helper methods instead of separate fields - Update all 22 geom implementations with new structure - Fix semantic bugs where MAPPING validation incorrectly included Delayed aesthetics Co-Authored-By: Claude Sonnet 4.5 --- src/execute/mod.rs | 26 +++++------ src/plot/layer/geom/abline.rs | 23 +++++----- src/plot/layer/geom/area.rs | 24 +++++----- src/plot/layer/geom/arrow.rs | 28 ++++++------ src/plot/layer/geom/bar.rs | 18 +++++--- src/plot/layer/geom/boxplot.rs | 48 ++++++++++---------- src/plot/layer/geom/density.rs | 25 +++++------ src/plot/layer/geom/errorbar.rs | 29 ++++++------ src/plot/layer/geom/histogram.rs | 20 ++++++--- src/plot/layer/geom/hline.rs | 17 +++++--- src/plot/layer/geom/label.rs | 24 ++++++---- src/plot/layer/geom/line.rs | 18 +++++--- src/plot/layer/geom/mod.rs | 14 +++--- src/plot/layer/geom/path.rs | 18 +++++--- src/plot/layer/geom/point.rs | 27 ++++++------ src/plot/layer/geom/polygon.rs | 25 +++++------ src/plot/layer/geom/ribbon.rs | 25 +++++------ src/plot/layer/geom/segment.rs | 27 ++++++------ src/plot/layer/geom/smooth.rs | 18 +++++--- src/plot/layer/geom/text.rs | 23 ++++++---- src/plot/layer/geom/tile.rs | 19 +++++--- src/plot/layer/geom/types.rs | 75 +++++++++++++++++++++++++++----- src/plot/layer/geom/violin.rs | 27 ++++++------ src/plot/layer/geom/vline.rs | 17 +++++--- src/plot/layer/mod.rs | 4 +- src/plot/main.rs | 55 ++++++++++++----------- src/plot/types.rs | 7 +++ src/writer/vegalite/mod.rs | 2 +- 28 files changed, 397 insertions(+), 286 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index cdd394ce..fd872a21 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -49,7 +49,7 @@ use crate::reader::DuckDBReader; fn validate(layers: &[Layer], layer_schemas: &[Schema]) -> Result<()> { for (idx, (layer, schema)) in layers.iter().zip(layer_schemas.iter()).enumerate() { let schema_columns: HashSet<&str> = schema.iter().map(|c| c.name.as_str()).collect(); - let supported = layer.geom.aesthetics().supported; + let supported = layer.geom.aesthetics().supported(); // Validate required aesthetics for this geom layer @@ -96,14 +96,10 @@ fn validate(layers: &[Layer], layer_schemas: &[Schema]) -> Result<()> { } // Validate remapping target aesthetics are supported by geom - // Target can be in supported OR hidden (hidden = valid REMAPPING targets but not MAPPING targets) + // REMAPPING can target any aesthetic (including Delayed ones from stat transforms) let aesthetics_info = layer.geom.aesthetics(); for target_aesthetic in layer.remappings.aesthetics.keys() { - let is_supported = aesthetics_info - .supported - .contains(&target_aesthetic.as_str()); - let is_hidden = aesthetics_info.hidden.contains(&target_aesthetic.as_str()); - if !is_supported && !is_hidden { + if !aesthetics_info.contains(target_aesthetic) { return Err(GgsqlError::ValidationError(format!( "Layer {}: REMAPPING targets unsupported aesthetic '{}' for geom '{}'", idx + 1, @@ -156,7 +152,7 @@ fn validate(layers: &[Layer], layer_schemas: &[Schema]) -> Result<()> { fn merge_global_mappings_into_layers(specs: &mut [Plot], layer_schemas: &[Schema]) { for spec in specs { for (layer, schema) in spec.layers.iter_mut().zip(layer_schemas.iter()) { - let supported = layer.geom.aesthetics().supported; + let supported = layer.geom.aesthetics().supported(); let schema_columns: HashSet<&str> = schema.iter().map(|c| c.name.as_str()).collect(); // 1. First merge explicit global aesthetics (layer overrides global) @@ -176,14 +172,14 @@ fn merge_global_mappings_into_layers(specs: &mut [Plot], layer_schemas: &[Schema // 2. Smart wildcard expansion: only expand to columns that exist in schema let has_wildcard = layer.mappings.wildcard || spec.global_mappings.wildcard; if has_wildcard { - for &aes in supported { + for aes in &supported { // Only create mapping if column exists in the schema - if schema_columns.contains(aes) { + if schema_columns.contains(*aes) { layer .mappings .aesthetics .entry(crate::parser::builder::normalise_aes_name(aes)) - .or_insert(AestheticValue::standard_column(aes)); + .or_insert(AestheticValue::standard_column(*aes)); } } } @@ -224,10 +220,10 @@ fn split_color_aesthetic(spec: &mut Plot) { // 2. Split color mapping to fill/stroke in layers, then remove color for layer in &mut spec.layers { if let Some(color_value) = layer.mappings.aesthetics.get("color").cloned() { - let supported = layer.geom.aesthetics().supported; + let aesthetics = layer.geom.aesthetics(); for &aes in &["stroke", "fill"] { - if supported.contains(&aes) { + if aesthetics.is_supported(aes) { layer .mappings .aesthetics @@ -244,10 +240,10 @@ fn split_color_aesthetic(spec: &mut Plot) { // 3. Split color parameter (SETTING) to fill/stroke in layers for layer in &mut spec.layers { if let Some(color_value) = layer.parameters.get("color").cloned() { - let supported = layer.geom.aesthetics().supported; + let aesthetics = layer.geom.aesthetics(); for &aes in &["stroke", "fill"] { - if supported.contains(&aes) { + if aesthetics.is_supported(aes) { layer .parameters .entry(aes.to_string()) diff --git a/src/plot/layer/geom/abline.rs b/src/plot/layer/geom/abline.rs index a630ea9e..6bdaefa5 100644 --- a/src/plot/layer/geom/abline.rs +++ b/src/plot/layer/geom/abline.rs @@ -1,6 +1,7 @@ //! AbLine geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// AbLine geom - lines with slope and intercept #[derive(Debug, Clone, Copy)] @@ -11,18 +12,16 @@ impl GeomTrait for AbLine { GeomType::AbLine } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "slope", - "intercept", - "stroke", - "linetype", - "linewidth", - "opacity", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("slope", DefaultAestheticValue::Required), + ("intercept", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), ], - required: &["slope", "intercept"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/area.rs b/src/plot/layer/geom/area.rs index f191bc16..ae9ecd1c 100644 --- a/src/plot/layer/geom/area.rs +++ b/src/plot/layer/geom/area.rs @@ -1,8 +1,8 @@ //! Area geom implementation -use crate::plot::{DefaultParam, DefaultParamValue}; +use crate::plot::{DefaultParam, DefaultParamValue, types::DefaultAestheticValue}; -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; /// Area geom - filled area charts #[derive(Debug, Clone, Copy)] @@ -13,19 +13,17 @@ impl GeomTrait for Area { GeomType::Area } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "fill", - "stroke", - "opacity", - "linewidth", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linewidth", DefaultAestheticValue::Null), // "linetype", // vegalite doesn't support strokeDash ], - required: &["x", "y"], - hidden: &[], } } diff --git a/src/plot/layer/geom/arrow.rs b/src/plot/layer/geom/arrow.rs index 2baa5396..9605512b 100644 --- a/src/plot/layer/geom/arrow.rs +++ b/src/plot/layer/geom/arrow.rs @@ -1,6 +1,7 @@ //! Arrow geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Arrow geom - line segments with arrowheads #[derive(Debug, Clone, Copy)] @@ -11,20 +12,19 @@ impl GeomTrait for Arrow { GeomType::Arrow } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "xend", - "yend", - "stroke", - "linetype", - "linewidth", - "opacity", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("xend", DefaultAestheticValue::Required), + ("yend", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::Null), ], - required: &["x", "y", "xend", "yend"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/bar.rs b/src/plot/layer/geom/bar.rs index 7ab7e2bf..f4091c8c 100644 --- a/src/plot/layer/geom/bar.rs +++ b/src/plot/layer/geom/bar.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use std::collections::HashSet; use super::types::get_column_name; -use super::{DefaultParam, DefaultParamValue, GeomAesthetics, GeomTrait, GeomType, StatResult}; +use super::{DefaultParam, DefaultParamValue, DefaultAesthetics, GeomTrait, GeomType, StatResult}; use crate::naming; use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::{DataFrame, GgsqlError, Mappings, Result}; @@ -20,15 +20,21 @@ impl GeomTrait for Bar { GeomType::Bar } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { // Bar supports optional x and y - stat decides aggregation // If x is missing: single bar showing total // If y is missing: stat computes COUNT or SUM(weight) // weight: optional, if mapped uses SUM(weight) instead of COUNT(*) - supported: &["x", "y", "weight", "fill", "stroke", "width", "opacity"], - required: &[], - hidden: &[], + defaults: &[ + ("x", DefaultAestheticValue::Null), // Optional - stat may provide + ("y", DefaultAestheticValue::Null), // Optional - stat may compute + ("weight", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::Null), + ("width", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(1.0)), + ], } } diff --git a/src/plot/layer/geom/boxplot.rs b/src/plot/layer/geom/boxplot.rs index 8a95ee85..5d36feb9 100644 --- a/src/plot/layer/geom/boxplot.rs +++ b/src/plot/layer/geom/boxplot.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; use crate::{ naming, plot::{ @@ -21,22 +21,22 @@ impl GeomTrait for Boxplot { GeomType::Boxplot } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "fill", - "stroke", - "opacity", - "linetype", - "linewidth", - "size", - "shape", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("fill", DefaultAestheticValue::String("#FFFFFF00")), // Transparent + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::Null), + ("size", DefaultAestheticValue::Null), + ("shape", DefaultAestheticValue::Null), + // Internal aesthetics produced by stat transform + ("type", DefaultAestheticValue::Delayed), + ("yend", DefaultAestheticValue::Delayed), ], - required: &["x", "y"], - // Internal aesthetics produced by stat transform - hidden: &["type", "y", "yend"], } } @@ -547,9 +547,9 @@ mod tests { let boxplot = Boxplot; let aes = boxplot.aesthetics(); - assert!(aes.required.contains(&"x")); - assert!(aes.required.contains(&"y")); - assert_eq!(aes.required.len(), 2); + assert!(aes.is_required("x")); + assert!(aes.is_required("y")); + assert_eq!(aes.required().len(), 2); } #[test] @@ -557,11 +557,11 @@ mod tests { let boxplot = Boxplot; let aes = boxplot.aesthetics(); - assert!(aes.supported.contains(&"x")); - assert!(aes.supported.contains(&"y")); - assert!(aes.supported.contains(&"fill")); - assert!(aes.supported.contains(&"stroke")); - assert!(aes.supported.contains(&"opacity")); + assert!(aes.is_supported("x")); + assert!(aes.is_supported("y")); + assert!(aes.is_supported("fill")); + assert!(aes.is_supported("stroke")); + assert!(aes.is_supported("opacity")); } #[test] diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index ff3c9e19..982d36e5 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -1,6 +1,6 @@ //! Density geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; use crate::{ naming, plot::{ @@ -24,19 +24,18 @@ impl GeomTrait for Density { GeomType::Density } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "weight", - "fill", - "stroke", - "opacity", - "linewidth", - "linetype", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("weight", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::String("black")), + ("opacity", DefaultAestheticValue::Number(0.5)), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ("y", DefaultAestheticValue::Delayed), // Computed by stat ], - required: &["x"], - hidden: &["y"], } } diff --git a/src/plot/layer/geom/errorbar.rs b/src/plot/layer/geom/errorbar.rs index eb007d60..3e23876c 100644 --- a/src/plot/layer/geom/errorbar.rs +++ b/src/plot/layer/geom/errorbar.rs @@ -1,6 +1,7 @@ //! ErrorBar geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// ErrorBar geom - error bars (confidence intervals) #[derive(Debug, Clone, Copy)] @@ -11,21 +12,19 @@ impl GeomTrait for ErrorBar { GeomType::ErrorBar } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "ymin", - "ymax", - "xmin", - "xmax", - "stroke", - "linewidth", - "opacity", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Null), + ("y", DefaultAestheticValue::Null), + ("ymin", DefaultAestheticValue::Null), + ("ymax", DefaultAestheticValue::Null), + ("xmin", DefaultAestheticValue::Null), + ("xmax", DefaultAestheticValue::Null), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), ], - required: &[], - hidden: &[], } } } diff --git a/src/plot/layer/geom/histogram.rs b/src/plot/layer/geom/histogram.rs index 2a4358ae..2ce7ad90 100644 --- a/src/plot/layer/geom/histogram.rs +++ b/src/plot/layer/geom/histogram.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use super::types::get_column_name; -use super::{DefaultParam, DefaultParamValue, GeomAesthetics, GeomTrait, GeomType, StatResult}; +use super::{DefaultParam, DefaultParamValue, DefaultAesthetics, GeomTrait, GeomType, StatResult}; use crate::naming; use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::{DataFrame, GgsqlError, Mappings, Result}; @@ -19,12 +19,18 @@ impl GeomTrait for Histogram { GeomType::Histogram } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["x", "weight", "fill", "stroke", "opacity"], - required: &["x"], - // y and xend are produced by stat_histogram but not valid for manual MAPPING - hidden: &["y", "xend"], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("weight", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(1.0)), + // y and xend are produced by stat_histogram but not valid for manual MAPPING + ("y", DefaultAestheticValue::Delayed), + ("xend", DefaultAestheticValue::Delayed), + ], } } diff --git a/src/plot/layer/geom/hline.rs b/src/plot/layer/geom/hline.rs index 1843cc19..825da478 100644 --- a/src/plot/layer/geom/hline.rs +++ b/src/plot/layer/geom/hline.rs @@ -1,6 +1,7 @@ //! HLine geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// HLine geom - horizontal reference lines #[derive(Debug, Clone, Copy)] @@ -11,11 +12,15 @@ impl GeomTrait for HLine { GeomType::HLine } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["yintercept", "stroke", "linetype", "linewidth", "opacity"], - required: &["yintercept"], - hidden: &[], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("yintercept", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ], } } } diff --git a/src/plot/layer/geom/label.rs b/src/plot/layer/geom/label.rs index 8481898a..e5c92111 100644 --- a/src/plot/layer/geom/label.rs +++ b/src/plot/layer/geom/label.rs @@ -1,6 +1,7 @@ //! Label geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Label geom - text labels with background #[derive(Debug, Clone, Copy)] @@ -11,14 +12,21 @@ impl GeomTrait for Label { GeomType::Label } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", "y", "label", "fill", "stroke", "size", "opacity", "family", "fontface", - "hjust", "vjust", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("label", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::Null), + ("stroke", DefaultAestheticValue::Null), + ("size", DefaultAestheticValue::Number(11.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("family", DefaultAestheticValue::Null), + ("fontface", DefaultAestheticValue::Null), + ("hjust", DefaultAestheticValue::Null), + ("vjust", DefaultAestheticValue::Null), ], - required: &["x", "y"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index f26d7494..2c8bd5b2 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -1,6 +1,7 @@ //! Line geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Line geom - line charts with connected points #[derive(Debug, Clone, Copy)] @@ -11,11 +12,16 @@ impl GeomTrait for Line { GeomType::Line } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["x", "y", "stroke", "linetype", "linewidth", "opacity"], - required: &["x", "y"], - hidden: &[], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ], } } } diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 0b34b245..eee995a0 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -17,7 +17,7 @@ //! //! let point = Geom::point(); //! assert_eq!(point.geom_type(), GeomType::Point); -//! assert!(point.aesthetics().required.contains(&"x")); +//! assert!(point.aesthetics().is_required("x")); //! ``` use crate::{DataFrame, Mappings, Result}; @@ -51,7 +51,7 @@ mod violin; mod vline; // Re-export types -pub use types::{DefaultParam, DefaultParamValue, GeomAesthetics, StatResult}; +pub use types::{DefaultParam, DefaultParamValue, DefaultAesthetics, StatResult}; // Re-export aesthetic family utilities from the central module pub use crate::plot::aesthetic::{get_aesthetic_family, AESTHETIC_FAMILIES}; @@ -146,7 +146,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { fn geom_type(&self) -> GeomType; /// Returns aesthetic information (REQUIRED - each geom is different) - fn aesthetics(&self) -> GeomAesthetics; + fn aesthetics(&self) -> DefaultAesthetics; /// Returns default remappings for stat-computed columns and literals to aesthetics. /// @@ -213,7 +213,7 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { /// /// Combines supported aesthetics with non-aesthetic parameters. fn valid_settings(&self) -> Vec<&'static str> { - let mut valid: Vec<&'static str> = self.aesthetics().supported.to_vec(); + let mut valid: Vec<&'static str> = self.aesthetics().supported(); for param in self.default_params() { valid.push(param.name); } @@ -367,7 +367,7 @@ impl Geom { } /// Get aesthetics information - pub fn aesthetics(&self) -> GeomAesthetics { + pub fn aesthetics(&self) -> DefaultAesthetics { self.0.aesthetics() } @@ -506,8 +506,8 @@ mod tests { fn test_geom_aesthetics() { let point = Geom::point(); let aes = point.aesthetics(); - assert!(aes.required.contains(&"x")); - assert!(aes.required.contains(&"y")); + assert!(aes.is_required("x")); + assert!(aes.is_required("y")); } #[test] diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index f289032c..5f46aed8 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -1,6 +1,7 @@ //! Path geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Path geom - connected line segments in order #[derive(Debug, Clone, Copy)] @@ -11,11 +12,16 @@ impl GeomTrait for Path { GeomType::Path } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["x", "y", "stroke", "linetype", "linewidth", "opacity"], - required: &["x", "y"], - hidden: &[], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ], } } } diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 0e925795..ba5e530e 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -1,6 +1,7 @@ //! Point geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Point geom - scatter plots and similar #[derive(Debug, Clone, Copy)] @@ -11,20 +12,18 @@ impl GeomTrait for Point { GeomType::Point } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "fill", - "stroke", - "size", - "shape", - "opacity", - "linewidth", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("size", DefaultAestheticValue::Number(3.0)), + ("stroke", DefaultAestheticValue::String("black")), + ("fill", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("shape", DefaultAestheticValue::Null), + ("linewidth", DefaultAestheticValue::Null), ], - required: &["x", "y"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index e232db34..aa160cb3 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -1,6 +1,7 @@ //! Polygon geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Polygon geom - arbitrary polygons #[derive(Debug, Clone, Copy)] @@ -11,19 +12,17 @@ impl GeomTrait for Polygon { GeomType::Polygon } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "fill", - "stroke", - "opacity", - "linewidth", - "linetype", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("fill", DefaultAestheticValue::String("#888888")), + ("stroke", DefaultAestheticValue::String("#888888")), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linewidth", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::Null), ], - required: &["x", "y"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index c6d62073..763481a8 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -1,6 +1,7 @@ //! Ribbon geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Ribbon geom - confidence bands and ranges #[derive(Debug, Clone, Copy)] @@ -11,20 +12,18 @@ impl GeomTrait for Ribbon { GeomType::Ribbon } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "ymin", - "ymax", - "fill", - "stroke", - "opacity", - "linewidth", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("ymin", DefaultAestheticValue::Required), + ("ymax", DefaultAestheticValue::Required), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(0.5)), + ("linewidth", DefaultAestheticValue::Null), // "linetype" // vegalite doesn't support strokeDash ], - required: &["x", "ymin", "ymax"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/segment.rs b/src/plot/layer/geom/segment.rs index 0c26cc02..c930a4c2 100644 --- a/src/plot/layer/geom/segment.rs +++ b/src/plot/layer/geom/segment.rs @@ -1,6 +1,7 @@ //! Segment geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Segment geom - line segments between two points #[derive(Debug, Clone, Copy)] @@ -11,20 +12,18 @@ impl GeomTrait for Segment { GeomType::Segment } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "xend", - "yend", - "stroke", - "linetype", - "linewidth", - "opacity", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("xend", DefaultAestheticValue::Required), + ("yend", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), ], - required: &["x", "y", "xend", "yend"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/smooth.rs b/src/plot/layer/geom/smooth.rs index 06243523..13aef8e9 100644 --- a/src/plot/layer/geom/smooth.rs +++ b/src/plot/layer/geom/smooth.rs @@ -1,6 +1,7 @@ //! Smooth geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; use crate::Mappings; /// Smooth geom - smoothed conditional means (regression, LOESS, etc.) @@ -12,11 +13,16 @@ impl GeomTrait for Smooth { GeomType::Smooth } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["x", "y", "stroke", "linetype", "opacity"], - required: &["x", "y"], - hidden: &[], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("blue")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ], } } diff --git a/src/plot/layer/geom/text.rs b/src/plot/layer/geom/text.rs index 7107f5c5..b63cb780 100644 --- a/src/plot/layer/geom/text.rs +++ b/src/plot/layer/geom/text.rs @@ -1,6 +1,7 @@ //! Text geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Text geom - text labels at positions #[derive(Debug, Clone, Copy)] @@ -11,14 +12,20 @@ impl GeomTrait for Text { GeomType::Text } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", "y", "label", "stroke", "size", "opacity", "family", "fontface", "hjust", - "vjust", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("label", DefaultAestheticValue::Null), + ("stroke", DefaultAestheticValue::Null), + ("size", DefaultAestheticValue::Number(11.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("family", DefaultAestheticValue::Null), + ("fontface", DefaultAestheticValue::Null), + ("hjust", DefaultAestheticValue::Null), + ("vjust", DefaultAestheticValue::Null), ], - required: &["x", "y"], - hidden: &[], } } } diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index effe2a89..2feb5903 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -1,6 +1,7 @@ //! Tile geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// Tile geom - heatmaps and tile-based visualizations #[derive(Debug, Clone, Copy)] @@ -11,11 +12,17 @@ impl GeomTrait for Tile { GeomType::Tile } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["x", "y", "fill", "stroke", "width", "height", "opacity"], - required: &["x", "y"], - hidden: &[], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::Null), + ("width", DefaultAestheticValue::Null), + ("height", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(1.0)), + ], } } } diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index 2d4ecc97..a6091169 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -2,20 +2,73 @@ //! //! These types are used by all geom implementations and are shared across the module. -use crate::Mappings; +use crate::{Mappings, plot::types::DefaultAestheticValue}; -/// Aesthetic information for a geom type +/// Default aesthetic values for a geom type /// -/// This struct describes which aesthetics a geom supports, requires, and hides. +/// This struct describes which aesthetics a geom supports, requires, and their default values. #[derive(Debug, Clone, Copy)] -pub struct GeomAesthetics { - /// All aesthetics this geom type supports for user MAPPING - pub supported: &'static [&'static str], - /// Aesthetics required for this geom type to be valid - pub required: &'static [&'static str], - /// Hidden aesthetics (valid REMAPPING targets, not valid MAPPING targets) - /// These are produced by stat transforms but shouldn't be manually mapped - pub hidden: &'static [&'static str], +pub struct DefaultAesthetics { + /// Aesthetic defaults: maps aesthetic name to default value + /// - Required: Must be provided via MAPPING + /// - Delayed: Produced by stat transform (REMAPPING only) + /// - Null: Supported but no default + /// - Other variants: Actual default values + pub defaults: &'static [(&'static str, DefaultAestheticValue)], +} + +impl DefaultAesthetics { + /// Get all aesthetic names (including Delayed) + pub fn names(&self) -> Vec<&'static str> { + self.defaults.iter().map(|(name, _)| *name).collect() + } + + /// Get supported aesthetic names (excludes Delayed, for MAPPING validation) + pub fn supported(&self) -> Vec<&'static str> { + self.defaults + .iter() + .filter_map(|(name, value)| { + if !matches!(value, DefaultAestheticValue::Delayed) { + Some(*name) + } else { + None + } + }) + .collect() + } + + /// Get required aesthetic names (those marked as Required) + pub fn required(&self) -> Vec<&'static str> { + self.defaults + .iter() + .filter_map(|(name, value)| { + if matches!(value, DefaultAestheticValue::Required) { + Some(*name) + } else { + None + } + }) + .collect() + } + + /// Check if an aesthetic is supported (not Delayed) + pub fn is_supported(&self, name: &str) -> bool { + self.defaults + .iter() + .any(|(n, value)| *n == name && !matches!(value, DefaultAestheticValue::Delayed)) + } + + /// Check if an aesthetic exists (including Delayed) + pub fn contains(&self, name: &str) -> bool { + self.defaults.iter().any(|(n, _)| *n == name) + } + + /// Check if an aesthetic is required + pub fn is_required(&self, name: &str) -> bool { + self.defaults + .iter() + .any(|(n, value)| *n == name && matches!(value, DefaultAestheticValue::Required)) + } } /// Default value for a layer parameter diff --git a/src/plot/layer/geom/violin.rs b/src/plot/layer/geom/violin.rs index 0335009e..df9b8336 100644 --- a/src/plot/layer/geom/violin.rs +++ b/src/plot/layer/geom/violin.rs @@ -1,6 +1,6 @@ //! Violin geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType, StatResult}; +use super::{DefaultAesthetics, GeomTrait, GeomType, StatResult}; use crate::{ plot::{ geom::types::get_column_name, DefaultAestheticValue, DefaultParam, DefaultParamValue, @@ -19,20 +19,19 @@ impl GeomTrait for Violin { GeomType::Violin } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &[ - "x", - "y", - "weight", - "fill", - "stroke", - "opacity", - "linewidth", - "linetype", + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("weight", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("steelblue")), + ("stroke", DefaultAestheticValue::String("black")), + ("opacity", DefaultAestheticValue::Number(0.7)), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ("offset", DefaultAestheticValue::Delayed), // Computed by stat ], - required: &["x", "y"], - hidden: &["offset"], } } diff --git a/src/plot/layer/geom/vline.rs b/src/plot/layer/geom/vline.rs index 84ff2bba..b0a68207 100644 --- a/src/plot/layer/geom/vline.rs +++ b/src/plot/layer/geom/vline.rs @@ -1,6 +1,7 @@ //! VLine geom implementation -use super::{GeomAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType}; +use crate::plot::types::DefaultAestheticValue; /// VLine geom - vertical reference lines #[derive(Debug, Clone, Copy)] @@ -11,11 +12,15 @@ impl GeomTrait for VLine { GeomType::VLine } - fn aesthetics(&self) -> GeomAesthetics { - GeomAesthetics { - supported: &["xintercept", "stroke", "linetype", "linewidth", "opacity"], - required: &["xintercept"], - hidden: &[], + fn aesthetics(&self) -> DefaultAesthetics { + DefaultAesthetics { + defaults: &[ + ("xintercept", DefaultAestheticValue::Required), + ("stroke", DefaultAestheticValue::String("black")), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::Null), + ], } } } diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index 5c9909f8..3a1ec047 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -11,7 +11,7 @@ pub mod geom; // Re-export geom types for convenience pub use geom::{ - DefaultParam, DefaultParamValue, Geom, GeomAesthetics, GeomTrait, GeomType, StatResult, + DefaultParam, DefaultParamValue, Geom, DefaultAesthetics, GeomTrait, GeomType, StatResult, }; use crate::plot::types::{AestheticValue, DataSource, Mappings, ParameterValue, SqlExpression}; @@ -119,7 +119,7 @@ impl Layer { /// Check if this layer has the required aesthetics for its geom pub fn validate_required_aesthetics(&self) -> std::result::Result<(), String> { - for aesthetic in self.geom.aesthetics().required { + for aesthetic in self.geom.aesthetics().required() { if !self.mappings.contains_key(aesthetic) { return Err(format!( "Geom '{}' requires aesthetic '{}' but it was not provided", diff --git a/src/plot/main.rs b/src/plot/main.rs index c51ff708..4f3bdfde 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -30,7 +30,7 @@ pub use super::types::{ // Re-export Geom and related types from the layer::geom module pub use super::layer::geom::{ - DefaultParam, DefaultParamValue, Geom, GeomAesthetics, GeomTrait, GeomType, StatResult, + DefaultAesthetics, DefaultParam, DefaultParamValue, Geom, GeomTrait, GeomType, StatResult, }; use super::aesthetic::primary_aesthetic; @@ -381,56 +381,59 @@ mod tests { fn test_geom_aesthetics() { // Point geom let point = Geom::point().aesthetics(); - assert!(point.supported.contains(&"x")); - assert!(point.supported.contains(&"size")); - assert!(point.supported.contains(&"shape")); - assert!(!point.supported.contains(&"linetype")); - assert_eq!(point.required, &["x", "y"]); + assert!(point.is_supported("x")); + assert!(point.is_supported("size")); + assert!(point.is_supported("shape")); + assert!(!point.is_supported("linetype")); + assert_eq!(point.required(), &["x", "y"]); // Line geom let line = Geom::line().aesthetics(); - assert!(line.supported.contains(&"linetype")); - assert!(line.supported.contains(&"linewidth")); - assert!(!line.supported.contains(&"size")); - assert_eq!(line.required, &["x", "y"]); + assert!(line.is_supported("linetype")); + assert!(line.is_supported("linewidth")); + assert!(!line.is_supported("size")); + assert_eq!(line.required(), &["x", "y"]); // Bar geom - optional x and y (stat decides aggregation) let bar = Geom::bar().aesthetics(); - assert!(bar.supported.contains(&"fill")); - assert!(bar.supported.contains(&"width")); - assert!(bar.supported.contains(&"y")); // Bar accepts optional y - assert!(bar.supported.contains(&"x")); // Bar accepts optional x - assert_eq!(bar.required, &[] as &[&str]); // No required aesthetics + assert!(bar.is_supported("fill")); + assert!(bar.is_supported("width")); + assert!(bar.is_supported("y")); // Bar accepts optional y + assert!(bar.is_supported("x")); // Bar accepts optional x + assert_eq!(bar.required(), &[] as &[&str]); // No required aesthetics // Text geom let text = Geom::text().aesthetics(); - assert!(text.supported.contains(&"label")); - assert!(text.supported.contains(&"family")); - assert_eq!(text.required, &["x", "y"]); + assert!(text.is_supported("label")); + assert!(text.is_supported("family")); + assert_eq!(text.required(), &["x", "y"]); // Statistical geoms only require x - assert_eq!(Geom::histogram().aesthetics().required, &["x"]); - assert_eq!(Geom::density().aesthetics().required, &["x"]); + assert_eq!(Geom::histogram().aesthetics().required(), &["x"]); + assert_eq!(Geom::density().aesthetics().required(), &["x"]); // Ribbon requires ymin/ymax - assert_eq!(Geom::ribbon().aesthetics().required, &["x", "ymin", "ymax"]); + assert_eq!( + Geom::ribbon().aesthetics().required(), + &["x", "ymin", "ymax"] + ); // Segment/arrow require endpoints assert_eq!( - Geom::segment().aesthetics().required, + Geom::segment().aesthetics().required(), &["x", "y", "xend", "yend"] ); // Reference lines - assert_eq!(Geom::hline().aesthetics().required, &["yintercept"]); - assert_eq!(Geom::vline().aesthetics().required, &["xintercept"]); + assert_eq!(Geom::hline().aesthetics().required(), &["yintercept"]); + assert_eq!(Geom::vline().aesthetics().required(), &["xintercept"]); assert_eq!( - Geom::abline().aesthetics().required, + Geom::abline().aesthetics().required(), &["slope", "intercept"] ); // ErrorBar has no strict requirements - assert_eq!(Geom::errorbar().aesthetics().required, &[] as &[&str]); + assert_eq!(Geom::errorbar().aesthetics().required(), &[] as &[&str]); } #[test] diff --git a/src/plot/types.rs b/src/plot/types.rs index ec1ce054..8c1b0357 100644 --- a/src/plot/types.rs +++ b/src/plot/types.rs @@ -255,6 +255,12 @@ pub enum DefaultAestheticValue { Number(f64), /// Literal boolean value Boolean(bool), + /// Supported but no default value (optional aesthetic) + Null, + /// Required aesthetic (must be provided via MAPPING) + Required, + /// Delayed aesthetic (produced by stat transform, valid for REMAPPING only, not MAPPING) + Delayed, } impl DefaultAestheticValue { @@ -265,6 +271,7 @@ impl DefaultAestheticValue { Self::String(s) => AestheticValue::Literal(ParameterValue::String(s.to_string())), Self::Number(n) => AestheticValue::Literal(ParameterValue::Number(*n)), Self::Boolean(b) => AestheticValue::Literal(ParameterValue::Boolean(*b)), + Self::Null | Self::Required | Self::Delayed => AestheticValue::Literal(ParameterValue::Null), } } } diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index e7d72da3..8a6a40b4 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -252,7 +252,7 @@ fn build_layer_encoding( // Add aesthetic parameters from SETTING as literal encodings // (e.g., SETTING color => 'red' becomes {"color": {"value": "red"}}) // Only parameters that are supported aesthetics for this geom type are included - let supported_aesthetics = layer.geom.aesthetics().supported; + let supported_aesthetics = layer.geom.aesthetics().supported(); for (param_name, param_value) in &layer.parameters { if supported_aesthetics.contains(¶m_name.as_str()) { let channel_name = map_aesthetic_name(param_name); From 32f2b84c5cc1b13630418412e9966b2b80b98e1e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 12:17:25 +0100 Subject: [PATCH 02/19] Apply default aesthetic values from geom definitions in writer Implement Step 4 of the plan: writer now applies default aesthetic values from each geom's DefaultAesthetics when building encodings. Defaults are applied with lowest priority (after MAPPING and SETTING). Key changes: - Add logic in build_layer_encoding() to iterate over geom defaults - Skip if encoding already set by MAPPING or SETTING - Convert DefaultAestheticValue to AestheticValue via to_aesthetic_value() - Reuse existing build_encoding_channel() for unit conversions - Skip Required, Null, and Delayed variants (no literal defaults to apply) Precedence order: MAPPING > SETTING > defaults (geom) Co-Authored-By: Claude Sonnet 4.5 --- src/writer/vegalite/mod.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 8a6a40b4..a9a71438 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -272,6 +272,31 @@ fn build_layer_encoding( } } + // Add default aesthetic values from geom (lowest priority) + // Only apply if not already set by MAPPING or SETTING + let geom_aesthetics = layer.geom.aesthetics(); + for (aesthetic_name, default_value) in geom_aesthetics.defaults { + let channel_name = map_aesthetic_name(aesthetic_name); + + // Skip if already set by MAPPING or SETTING + if encoding.contains_key(&channel_name) { + continue; + } + + // Convert to AestheticValue + let aesthetic_value = default_value.to_aesthetic_value(); + + // Skip if not a literal or if it's Null (Required/Null/Delayed all become Null) + match aesthetic_value { + AestheticValue::Literal(ParameterValue::Null) | AestheticValue::Column { .. } => continue, + AestheticValue::Literal(_) => {} // Has a real value, proceed + } + + // Use existing encoding builder with unit conversions + let channel_encoding = build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; + encoding.insert(channel_name, channel_encoding); + } + // Add detail encoding for partition_by columns (grouping) if let Some(detail) = build_detail_encoding(&layer.partition_by) { encoding.insert("detail".to_string(), detail); From 7d508e3b1ca059cae40fb0d8371f12faa54e530d Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 12:34:53 +0100 Subject: [PATCH 03/19] Remove hardcoded aesthetic defaults from all renderers Remove redundant hardcoded defaults from BoxplotRenderer and PolygonRenderer. These defaults are now handled by the centralized default aesthetics system in build_layer_encoding(). Changes: - BoxplotRenderer: removed default_stroke, default_fill, default_linewidth - PolygonRenderer: removed hardcoded fill and stroke ("#888888") Encoding-level properties (from geom defaults) override mark-level properties, so the hardcoded values were redundant. All defaults now come from each geom's DefaultAesthetics definition. Co-Authored-By: Claude Sonnet 4.5 --- src/writer/vegalite/layer.rs | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index 00fe0ae4..ee2a225a 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -297,9 +297,7 @@ impl GeomRenderer for PolygonRenderer { fn modify_spec(&self, layer_spec: &mut Value, _layer: &Layer) -> Result<()> { layer_spec["mark"] = json!({ "type": "line", - "interpolate": "linear-closed", - "fill": "#888888", - "stroke": "#888888" + "interpolate": "linear-closed" }); Ok(()) } @@ -583,11 +581,6 @@ impl BoxplotRenderer { width = *num; } - // Default styling - let default_stroke = "black"; - let default_fill = "#FFFFFF00"; - let default_linewidth = 1.0; - // Helper to create filter transform for source selection let make_source_filter = |type_suffix: &str| -> Value { let source_key = format!("{}{}", base_key, type_suffix); @@ -620,9 +613,7 @@ impl BoxplotRenderer { &prototype, "outlier", json!({ - "type": "point", - "stroke": default_stroke, - "strokeWidth": default_linewidth + "type": "point" }), ); if points["encoding"].get("color").is_some() { @@ -656,9 +647,7 @@ impl BoxplotRenderer { &summary_prototype, "lower_whisker", json!({ - "type": "rule", - "stroke": default_stroke, - "size": default_linewidth + "type": "rule" }), ); @@ -678,9 +667,7 @@ impl BoxplotRenderer { &summary_prototype, "upper_whisker", json!({ - "type": "rule", - "stroke": default_stroke, - "size": default_linewidth + "type": "rule" }), ); @@ -702,10 +689,7 @@ impl BoxplotRenderer { json!({ "type": "bar", "width": {"band": width}, - "align": "center", - "stroke": default_stroke, - "color": default_fill, - "strokeWidth": default_linewidth + "align": "center" }), ); box_part["encoding"][value_var1] = y_encoding.clone(); @@ -717,10 +701,8 @@ impl BoxplotRenderer { "median", json!({ "type": "tick", - "stroke": default_stroke, "width": {"band": width}, - "align": "center", - "strokeWidth": default_linewidth + "align": "center" }), ); median_line["encoding"][value_var1] = y_encoding; From 2e1f0bb1fb28aa49dbc31ea5aadb538f1ace23e4 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 13:57:58 +0100 Subject: [PATCH 04/19] Add comprehensive tests for default aesthetic system Add unit and integration tests to verify default aesthetic behavior: Unit tests (types.rs): - test_default_aesthetics_methods: Comprehensive test of all helper methods (get, names, supported, required, is_supported, contains, is_required) on a DefaultAesthetics instance with various value types Integration tests (vegalite/mod.rs): - test_default_aesthetics_applied: Verify defaults appear in Vega-Lite output (stroke="black", opacity=1.0) - test_setting_overrides_default: Verify SETTING takes precedence over defaults - test_mapping_overrides_default: Verify MAPPING takes precedence over defaults - test_null_defaults_not_applied: Verify Null variants don't create encodings Also adds get() helper method to DefaultAesthetics for cleaner test code. Co-Authored-By: Claude Sonnet 4.5 --- src/plot/layer/geom/types.rs | 77 +++++++++++++ src/writer/vegalite/mod.rs | 213 ++++++++++++++++++++++++++++++++++- 2 files changed, 289 insertions(+), 1 deletion(-) diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index a6091169..804cc920 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -69,6 +69,14 @@ impl DefaultAesthetics { .iter() .any(|(n, value)| *n == name && matches!(value, DefaultAestheticValue::Required)) } + + /// Get the default value for an aesthetic by name + pub fn get(&self, name: &str) -> Option<&'static DefaultAestheticValue> { + self.defaults + .iter() + .find(|(n, _)| *n == name) + .map(|(_, value)| value) + } } /// Default value for a layer parameter @@ -126,3 +134,72 @@ pub fn get_column_name(aesthetics: &Mappings, aesthetic: &str) -> Option _ => None, }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_default_aesthetics_methods() { + // Create a DefaultAesthetics with various value types + let aes = DefaultAesthetics { + defaults: &[ + ("x", DefaultAestheticValue::Required), + ("y", DefaultAestheticValue::Required), + ("size", DefaultAestheticValue::Number(3.0)), + ("stroke", DefaultAestheticValue::String("black")), + ("fill", DefaultAestheticValue::Null), + ("yend", DefaultAestheticValue::Delayed), + ], + }; + + // Test get() method + assert_eq!(aes.get("x"), Some(&DefaultAestheticValue::Required)); + assert_eq!(aes.get("size"), Some(&DefaultAestheticValue::Number(3.0))); + assert_eq!( + aes.get("stroke"), + Some(&DefaultAestheticValue::String("black")) + ); + assert_eq!(aes.get("fill"), Some(&DefaultAestheticValue::Null)); + assert_eq!(aes.get("yend"), Some(&DefaultAestheticValue::Delayed)); + assert_eq!(aes.get("nonexistent"), None); + + // Test names() - includes all aesthetics + let names = aes.names(); + assert_eq!(names.len(), 6); + assert!(names.contains(&"x")); + assert!(names.contains(&"yend")); + + // Test supported() - excludes Delayed + let supported = aes.supported(); + assert_eq!(supported.len(), 5); + assert!(supported.contains(&"x")); + assert!(supported.contains(&"size")); + assert!(supported.contains(&"fill")); + assert!(!supported.contains(&"yend")); // Delayed excluded + + // Test required() - only Required variants + let required = aes.required(); + assert_eq!(required.len(), 2); + assert!(required.contains(&"x")); + assert!(required.contains(&"y")); + assert!(!required.contains(&"size")); + + // Test is_supported() - efficient membership check + assert!(aes.is_supported("x")); + assert!(aes.is_supported("size")); + assert!(!aes.is_supported("yend")); // Delayed not supported + assert!(!aes.is_supported("nonexistent")); + + // Test contains() - includes Delayed + assert!(aes.contains("x")); + assert!(aes.contains("yend")); // Delayed included + assert!(!aes.contains("nonexistent")); + + // Test is_required() + assert!(aes.is_required("x")); + assert!(aes.is_required("y")); + assert!(!aes.is_required("size")); + assert!(!aes.is_required("yend")); + } +} diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index a9a71438..8845f147 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -28,6 +28,7 @@ mod layer; // ArrayElement is used in tests and for pattern matching; suppress unused import warning #[allow(unused_imports)] use crate::plot::ArrayElement; +use crate::plot::scale::linetype_to_stroke_dash; use crate::plot::ParameterValue; use crate::writer::Writer; use crate::{ @@ -258,12 +259,16 @@ fn build_layer_encoding( let channel_name = map_aesthetic_name(param_name); // Only add if not already set by MAPPING (MAPPING takes precedence) if !encoding.contains_key(&channel_name) { - // Convert size and linewidth from points to Vega-Lite units + // Convert size, linewidth, and linetype to Vega-Lite formats let converted_value = match (param_name.as_str(), param_value) { // Size: interpret as radius in points, convert to area in pixels^2 ("size", ParameterValue::Number(n)) => json!(n * n * POINTS_TO_AREA), // Linewidth: interpret as width in points, convert to pixels ("linewidth", ParameterValue::Number(n)) => json!(n * POINTS_TO_PIXELS), + // Linetype: convert string patterns to strokeDash arrays + ("linetype", ParameterValue::String(s)) => { + linetype_to_stroke_dash(s).map(|arr| json!(arr)).unwrap_or_else(|| json!(s)) + } // Other aesthetics: pass through unchanged _ => param_value.to_json(), }; @@ -587,6 +592,52 @@ mod tests { data_map } + /// Helper to build a layer with x and y aesthetics already set up + /// + /// By default, maps "x" column to x aesthetic and "y" column to y aesthetic. + /// Additional aesthetics and parameters can be added via builder methods. + /// + /// # Example + /// ``` + /// let layer = build_layer(Geom::point()) + /// .with_aesthetic("color".to_string(), AestheticValue::standard_column("category".to_string())); + /// ``` + fn build_layer(geom: Geom) -> Layer { + Layer::new(geom) + .with_aesthetic( + "x".to_string(), + AestheticValue::standard_column("x".to_string()), + ) + .with_aesthetic( + "y".to_string(), + AestheticValue::standard_column("y".to_string()), + ) + } + + /// Helper to build a complete spec with a single layer + /// + /// Creates a Plot with one layer that has x and y aesthetics mapped to "x" and "y" columns. + /// Additional aesthetics and parameters can be added to the layer before calling this. + /// + /// # Example + /// ``` + /// let spec = build_spec(Geom::line()); + /// ``` + fn build_spec(geom: Geom) -> Plot { + let mut spec = Plot::new(); + spec.layers.push(build_layer(geom)); + spec + } + + /// Helper to create a simple DataFrame with x and y columns for testing + fn simple_df() -> DataFrame { + df! { + "x" => &[1, 2, 3], + "y" => &[4, 5, 6], + } + .unwrap() + } + #[test] fn test_geom_to_mark_mapping() { // All marks should be objects with type and clip: true @@ -1063,4 +1114,164 @@ mod tests { "Last bin with closed='right' should use '> lower' format" ); } + + #[test] + fn test_default_aesthetics_applied() { + let writer = VegaLiteWriter::new(); + + // Point geom without explicit size/stroke - should use defaults + let spec = build_spec(Geom::point()); + + let result = writer.write(&spec, &wrap_data(simple_df())); + assert!(result.is_ok()); + let json_str = result.unwrap(); + let json: Value = serde_json::from_str(&json_str).unwrap(); + + // Single-layer spec uses layer array structure + let encoding = &json["layer"][0]["encoding"]; + + // Point default stroke = "black" + assert_eq!(encoding["stroke"]["value"], "black"); + + // Point default opacity = 1.0 + assert_eq!(encoding["opacity"]["value"], 1.0); + } + + #[test] + fn test_setting_overrides_default() { + let writer = VegaLiteWriter::new(); + + // Point with SETTING opacity => 0.5 should override default (1.0) + let mut spec = Plot::new(); + let layer = build_layer(Geom::point()) + .with_parameter("opacity".to_string(), ParameterValue::Number(0.5)); + spec.layers.push(layer); + + let result = writer.write(&spec, &wrap_data(simple_df())); + assert!(result.is_ok()); + let json_str = result.unwrap(); + let json: Value = serde_json::from_str(&json_str).unwrap(); + + let encoding = &json["layer"][0]["encoding"]; + + // Should use SETTING value (0.5), not default (1.0) + assert_eq!(encoding["opacity"]["value"], 0.5); + } + + #[test] + fn test_mapping_overrides_default() { + let writer = VegaLiteWriter::new(); + + // Point with MAPPING stroke AS stroke should override default + let mut spec = Plot::new(); + let layer = Layer::new(Geom::point()) + .with_aesthetic( + "x".to_string(), + AestheticValue::standard_column("x".to_string()), + ) + .with_aesthetic( + "y".to_string(), + AestheticValue::standard_column("y".to_string()), + ) + .with_aesthetic( + "stroke".to_string(), + AestheticValue::standard_column("stroke".to_string()), + ); + spec.layers.push(layer); + + let df = df! { + "x" => &[1, 2, 3], + "y" => &[4, 5, 6], + "stroke" => &["red", "blue", "green"], + } + .unwrap(); + + let result = writer.write(&spec, &wrap_data(df)); + assert!(result.is_ok()); + let json_str = result.unwrap(); + let json: Value = serde_json::from_str(&json_str).unwrap(); + + let encoding = &json["layer"][0]["encoding"]; + + // Should have field encoding, not value encoding + assert!(encoding["stroke"]["field"].is_string()); + assert_eq!(encoding["stroke"]["field"], "stroke"); + assert!(encoding["stroke"]["value"].is_null()); + } + + #[test] + fn test_null_defaults_not_applied() { + let writer = VegaLiteWriter::new(); + + // Point has linetype as Null - should not appear in encoding + let mut spec = Plot::new(); + let layer = Layer::new(Geom::point()) + .with_aesthetic( + "x".to_string(), + AestheticValue::standard_column("x".to_string()), + ) + .with_aesthetic( + "y".to_string(), + AestheticValue::standard_column("y".to_string()), + ); + spec.layers.push(layer); + + let df = df! { + "x" => &[1, 2, 3], + "y" => &[4, 5, 6], + } + .unwrap(); + + let result = writer.write(&spec, &wrap_data(df)); + assert!(result.is_ok()); + let json_str = result.unwrap(); + let json: Value = serde_json::from_str(&json_str).unwrap(); + + // Point has linetype => Null, should not appear in encoding + assert!(json["encoding"]["strokeDash"].is_null()); + } + + #[test] + fn test_linetype_translated_to_stroke_dash() { + let writer = VegaLiteWriter::new(); + + // Line with linetype as SETTING (literal) + let mut spec = Plot::new(); + let layer = build_layer(Geom::line()) + .with_aesthetic( + "linetype".to_string(), + AestheticValue::Literal(ParameterValue::String("dashed".to_string())), + ); + spec.layers.push(layer); + + let result = writer.write(&spec, &wrap_data(simple_df())); + assert!(result.is_ok()); + let json_str = result.unwrap(); + let json: Value = serde_json::from_str(&json_str).unwrap(); + + let encoding = &json["layer"][0]["encoding"]; + + // "dashed" should translate to [6, 4] + assert!(encoding["strokeDash"]["value"].is_array()); + assert_eq!(encoding["strokeDash"]["value"], json!([6, 4])); + } + + #[test] + fn test_linetype_default_translated_to_stroke_dash() { + let writer = VegaLiteWriter::new(); + + // Line geom has linetype default of "solid" + let spec = build_spec(Geom::line()); + + let result = writer.write(&spec, &wrap_data(simple_df())); + assert!(result.is_ok()); + let json_str = result.unwrap(); + let json: Value = serde_json::from_str(&json_str).unwrap(); + + let encoding = &json["layer"][0]["encoding"]; + + // "solid" should translate to empty array [] + assert!(encoding["strokeDash"]["value"].is_array()); + assert_eq!(encoding["strokeDash"]["value"], json!([])); + } } From b1cc361200336396a5d9dacd0079fefe508e7f06 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 15:34:18 +0100 Subject: [PATCH 05/19] Translate linetype strings to strokeDash arrays in writer. Use existing linetype_to_stroke_dash() function to convert linetype strings to Vega-Lite strokeDash arrays in two places: - SETTING parameters (e.g., SETTING linetype => 'dashed') - Default aesthetic values (e.g., line geom's default "solid") Named patterns like "solid", "dashed", "dotted" are converted to numeric arrays ([6,4] for dashed), while unrecognized patterns pass through as strings. Co-Authored-By: Claude Sonnet 4.5 --- src/writer/vegalite/encoding.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/writer/vegalite/encoding.rs b/src/writer/vegalite/encoding.rs index f489d2d7..ea0124e7 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -858,7 +858,10 @@ fn build_column_encoding( /// Build encoding for a literal aesthetic value fn build_literal_encoding(aesthetic: &str, lit: &ParameterValue) -> Result { let val = match lit { - ParameterValue::String(s) => json!(s), + ParameterValue::String(s) => match aesthetic { + "linetype" => linetype_to_stroke_dash(s).map(|arr| json!(arr)).unwrap_or_else(|| json!(s)), + _ => json!(s), + }, ParameterValue::Number(n) => { match aesthetic { // Size: radius (points) → area (pixels²) From 525b5713a64d6c45f38e42f0b323f9b7ff77fe5e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 16:15:02 +0100 Subject: [PATCH 06/19] Adjust some aesthetics to taste --- doc/syntax/layer/density.qmd | 12 ++++-------- doc/syntax/layer/point.qmd | 2 -- src/plot/layer/geom/abline.rs | 2 +- src/plot/layer/geom/area.rs | 12 ++++++------ src/plot/layer/geom/arrow.rs | 2 +- src/plot/layer/geom/bar.rs | 10 ++++++---- src/plot/layer/geom/boxplot.rs | 10 +++++----- src/plot/layer/geom/density.rs | 10 +++++----- src/plot/layer/geom/histogram.rs | 6 +++--- src/plot/layer/geom/hline.rs | 2 +- src/plot/layer/geom/line.rs | 4 ++-- src/plot/layer/geom/path.rs | 4 ++-- src/plot/layer/geom/point.rs | 6 +++--- src/plot/layer/geom/polygon.rs | 8 ++++---- src/plot/layer/geom/ribbon.rs | 10 +++++----- src/plot/layer/geom/segment.rs | 2 +- src/plot/layer/geom/smooth.rs | 4 ++-- src/plot/layer/geom/tile.rs | 4 ++-- src/plot/layer/geom/violin.rs | 8 ++++---- src/plot/layer/geom/vline.rs | 2 +- 20 files changed, 58 insertions(+), 62 deletions(-) diff --git a/doc/syntax/layer/density.qmd b/doc/syntax/layer/density.qmd index 8b8500f0..97319e31 100644 --- a/doc/syntax/layer/density.qmd +++ b/doc/syntax/layer/density.qmd @@ -73,14 +73,14 @@ A typical KDE computation with different groups: ```{ggsql} VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins - DRAW density SETTING opacity => 0.8 + DRAW density ``` Changing the relative bandwidth through the `adjust` setting. ```{ggsql} VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins - DRAW density SETTING opacity => 0.8, adjust => 0.1 + DRAW density SETTING adjust => 0.1 ``` Stacking the different groups instead of overlaying them. @@ -94,9 +94,7 @@ Using weighted estimates by mapping a column to the optional weight aesthetic. N ```{ggsql} VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins - DRAW density - MAPPING body_mass AS weight - SETTING opacity => 0.8 + DRAW density MAPPING body_mass AS weight ``` If you want to compare a histogram and a density layer, you can use the `intensity` computed variable to match the histogram scale. @@ -114,8 +112,6 @@ Note the relative height of the groups. ```{ggsql} VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins - DRAW density - REMAPPING intensity AS y - SETTING opacity => 0.8 + DRAW density REMAPPING intensity AS y ``` diff --git a/doc/syntax/layer/point.qmd b/doc/syntax/layer/point.qmd index f8e7b887..a1f181cb 100644 --- a/doc/syntax/layer/point.qmd +++ b/doc/syntax/layer/point.qmd @@ -35,7 +35,6 @@ Create a classic scatterplot VISUALISE FROM ggsql:penguins DRAW point MAPPING bill_len AS x, bill_dep AS y, species AS fill - SETTING size => 30 ``` Map to size to create a bubble chart @@ -52,6 +51,5 @@ Use filter to only plot a subset of the data VISUALISE FROM ggsql:penguins DRAW point MAPPING bill_len AS x, bill_dep AS y, species AS fill - SETTING size => 30 FILTER sex = 'female' ``` diff --git a/src/plot/layer/geom/abline.rs b/src/plot/layer/geom/abline.rs index 6bdaefa5..ff335dc6 100644 --- a/src/plot/layer/geom/abline.rs +++ b/src/plot/layer/geom/abline.rs @@ -20,7 +20,7 @@ impl GeomTrait for AbLine { ("stroke", DefaultAestheticValue::String("black")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/area.rs b/src/plot/layer/geom/area.rs index ae9ecd1c..5df42c4c 100644 --- a/src/plot/layer/geom/area.rs +++ b/src/plot/layer/geom/area.rs @@ -1,6 +1,6 @@ //! Area geom implementation -use crate::plot::{DefaultParam, DefaultParamValue, types::DefaultAestheticValue}; +use crate::plot::{types::DefaultAestheticValue, DefaultParam, DefaultParamValue}; use super::{DefaultAesthetics, GeomTrait, GeomType}; @@ -18,11 +18,11 @@ impl GeomTrait for Area { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("steelblue")), - ("stroke", DefaultAestheticValue::Null), - ("opacity", DefaultAestheticValue::Number(1.0)), - ("linewidth", DefaultAestheticValue::Null), - // "linetype", // vegalite doesn't support strokeDash + ("fill", DefaultAestheticValue::String("black")), + ("stroke", DefaultAestheticValue::String("black")), + ("opacity", DefaultAestheticValue::Number(0.8)), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/arrow.rs b/src/plot/layer/geom/arrow.rs index 9605512b..673a51dd 100644 --- a/src/plot/layer/geom/arrow.rs +++ b/src/plot/layer/geom/arrow.rs @@ -22,7 +22,7 @@ impl GeomTrait for Arrow { ("stroke", DefaultAestheticValue::String("black")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ("fill", DefaultAestheticValue::Null), ], } diff --git a/src/plot/layer/geom/bar.rs b/src/plot/layer/geom/bar.rs index f4091c8c..3bbce5f0 100644 --- a/src/plot/layer/geom/bar.rs +++ b/src/plot/layer/geom/bar.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use std::collections::HashSet; use super::types::get_column_name; -use super::{DefaultParam, DefaultParamValue, DefaultAesthetics, GeomTrait, GeomType, StatResult}; +use super::{DefaultAesthetics, DefaultParam, DefaultParamValue, GeomTrait, GeomType, StatResult}; use crate::naming; use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::{DataFrame, GgsqlError, Mappings, Result}; @@ -26,13 +26,15 @@ impl GeomTrait for Bar { // If x is missing: single bar showing total // If y is missing: stat computes COUNT or SUM(weight) // weight: optional, if mapped uses SUM(weight) instead of COUNT(*) + // width is a parameter, not an aesthetic. + // if we ever want to make 'width' an aesthetic, we'd probably need to + // translate it to 'size'. defaults: &[ ("x", DefaultAestheticValue::Null), // Optional - stat may provide ("y", DefaultAestheticValue::Null), // Optional - stat may compute ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("steelblue")), - ("stroke", DefaultAestheticValue::Null), - ("width", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("#000000B2")), + ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(1.0)), ], } diff --git a/src/plot/layer/geom/boxplot.rs b/src/plot/layer/geom/boxplot.rs index 5d36feb9..dbae53ea 100644 --- a/src/plot/layer/geom/boxplot.rs +++ b/src/plot/layer/geom/boxplot.rs @@ -27,12 +27,12 @@ impl GeomTrait for Boxplot { ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), ("stroke", DefaultAestheticValue::String("black")), - ("fill", DefaultAestheticValue::String("#FFFFFF00")), // Transparent + ("fill", DefaultAestheticValue::String("#FFFFFFCC")), ("linewidth", DefaultAestheticValue::Number(1.0)), - ("opacity", DefaultAestheticValue::Null), - ("linetype", DefaultAestheticValue::Null), - ("size", DefaultAestheticValue::Null), - ("shape", DefaultAestheticValue::Null), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::String("solid")), + ("size", DefaultAestheticValue::Number(3.0)), + ("shape", DefaultAestheticValue::String("circle")), // Internal aesthetics produced by stat transform ("type", DefaultAestheticValue::Delayed), ("yend", DefaultAestheticValue::Delayed), diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index 982d36e5..30f3afde 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -29,12 +29,12 @@ impl GeomTrait for Density { defaults: &[ ("x", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("steelblue")), - ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(0.5)), + ("fill", DefaultAestheticValue::String("black")), + ("stroke", DefaultAestheticValue::String("#000000B2")), + ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), - ("y", DefaultAestheticValue::Delayed), // Computed by stat + ("linetype", DefaultAestheticValue::String("solid")), + ("y", DefaultAestheticValue::Delayed), // Computed by stat ], } } diff --git a/src/plot/layer/geom/histogram.rs b/src/plot/layer/geom/histogram.rs index 2ce7ad90..205ff661 100644 --- a/src/plot/layer/geom/histogram.rs +++ b/src/plot/layer/geom/histogram.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use super::types::get_column_name; -use super::{DefaultParam, DefaultParamValue, DefaultAesthetics, GeomTrait, GeomType, StatResult}; +use super::{DefaultAesthetics, DefaultParam, DefaultParamValue, GeomTrait, GeomType, StatResult}; use crate::naming; use crate::plot::types::{DefaultAestheticValue, ParameterValue}; use crate::{DataFrame, GgsqlError, Mappings, Result}; @@ -24,8 +24,8 @@ impl GeomTrait for Histogram { defaults: &[ ("x", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("steelblue")), - ("stroke", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("#000000B2")), + ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(1.0)), // y and xend are produced by stat_histogram but not valid for manual MAPPING ("y", DefaultAestheticValue::Delayed), diff --git a/src/plot/layer/geom/hline.rs b/src/plot/layer/geom/hline.rs index 825da478..d6ad6951 100644 --- a/src/plot/layer/geom/hline.rs +++ b/src/plot/layer/geom/hline.rs @@ -19,7 +19,7 @@ impl GeomTrait for HLine { ("stroke", DefaultAestheticValue::String("black")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index 2c8bd5b2..c8c58494 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -18,9 +18,9 @@ impl GeomTrait for Line { ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), ("stroke", DefaultAestheticValue::String("black")), - ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linewidth", DefaultAestheticValue::Number(1.5)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/path.rs b/src/plot/layer/geom/path.rs index 5f46aed8..543ae821 100644 --- a/src/plot/layer/geom/path.rs +++ b/src/plot/layer/geom/path.rs @@ -18,9 +18,9 @@ impl GeomTrait for Path { ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), ("stroke", DefaultAestheticValue::String("black")), - ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linewidth", DefaultAestheticValue::Number(1.5)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index ba5e530e..9b4b18fd 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -19,10 +19,10 @@ impl GeomTrait for Point { ("y", DefaultAestheticValue::Required), ("size", DefaultAestheticValue::Number(3.0)), ("stroke", DefaultAestheticValue::String("black")), - ("fill", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("#000000B2")), ("opacity", DefaultAestheticValue::Number(1.0)), - ("shape", DefaultAestheticValue::Null), - ("linewidth", DefaultAestheticValue::Null), + ("shape", DefaultAestheticValue::String("circle")), + ("linewidth", DefaultAestheticValue::Number(1.0)), ], } } diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index aa160cb3..50804a68 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -17,11 +17,11 @@ impl GeomTrait for Polygon { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("#888888")), - ("stroke", DefaultAestheticValue::String("#888888")), + ("fill", DefaultAestheticValue::String("#000000B2")), + ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linewidth", DefaultAestheticValue::Null), - ("linetype", DefaultAestheticValue::Null), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 763481a8..76b7b717 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -18,11 +18,11 @@ impl GeomTrait for Ribbon { ("x", DefaultAestheticValue::Required), ("ymin", DefaultAestheticValue::Required), ("ymax", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("steelblue")), - ("stroke", DefaultAestheticValue::Null), - ("opacity", DefaultAestheticValue::Number(0.5)), - ("linewidth", DefaultAestheticValue::Null), - // "linetype" // vegalite doesn't support strokeDash + ("fill", DefaultAestheticValue::String("#000000B2")), + ("stroke", DefaultAestheticValue::String("black")), + ("opacity", DefaultAestheticValue::Number(1.0)), + ("linewidth", DefaultAestheticValue::Number(1.0)), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/segment.rs b/src/plot/layer/geom/segment.rs index c930a4c2..163ca795 100644 --- a/src/plot/layer/geom/segment.rs +++ b/src/plot/layer/geom/segment.rs @@ -22,7 +22,7 @@ impl GeomTrait for Segment { ("stroke", DefaultAestheticValue::String("black")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/smooth.rs b/src/plot/layer/geom/smooth.rs index 13aef8e9..ce0104ea 100644 --- a/src/plot/layer/geom/smooth.rs +++ b/src/plot/layer/geom/smooth.rs @@ -18,10 +18,10 @@ impl GeomTrait for Smooth { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("stroke", DefaultAestheticValue::String("blue")), + ("stroke", DefaultAestheticValue::String("#3366FF")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } diff --git a/src/plot/layer/geom/tile.rs b/src/plot/layer/geom/tile.rs index 2feb5903..133eaf3d 100644 --- a/src/plot/layer/geom/tile.rs +++ b/src/plot/layer/geom/tile.rs @@ -17,8 +17,8 @@ impl GeomTrait for Tile { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("steelblue")), - ("stroke", DefaultAestheticValue::Null), + ("fill", DefaultAestheticValue::String("black")), + ("stroke", DefaultAestheticValue::String("black")), ("width", DefaultAestheticValue::Null), ("height", DefaultAestheticValue::Null), ("opacity", DefaultAestheticValue::Number(1.0)), diff --git a/src/plot/layer/geom/violin.rs b/src/plot/layer/geom/violin.rs index df9b8336..3a0e342e 100644 --- a/src/plot/layer/geom/violin.rs +++ b/src/plot/layer/geom/violin.rs @@ -25,12 +25,12 @@ impl GeomTrait for Violin { ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("steelblue")), + ("fill", DefaultAestheticValue::String("#000000B2")), ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(0.7)), + ("opacity", DefaultAestheticValue::Number(1.0)), ("linewidth", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), - ("offset", DefaultAestheticValue::Delayed), // Computed by stat + ("linetype", DefaultAestheticValue::String("solid")), + ("offset", DefaultAestheticValue::Delayed), // Computed by stat ], } } diff --git a/src/plot/layer/geom/vline.rs b/src/plot/layer/geom/vline.rs index b0a68207..2b12cf1d 100644 --- a/src/plot/layer/geom/vline.rs +++ b/src/plot/layer/geom/vline.rs @@ -19,7 +19,7 @@ impl GeomTrait for VLine { ("stroke", DefaultAestheticValue::String("black")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::Null), + ("linetype", DefaultAestheticValue::String("solid")), ], } } From beb28bdd75cd2093707dd50a617cc3e92c378a28 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 16:54:12 +0100 Subject: [PATCH 07/19] Refactor SETTING and defaults processing in Vega-Lite writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unify SETTING parameter and default aesthetic processing into a single loop to eliminate code duplication and clarify precedence order. Changes: - Merge separate SETTING and defaults loops into unified loop - Use build_encoding_channel() for both (eliminates conversion duplication) - Make precedence explicit: SETTING > defaults (both checked per aesthetic) - Remove linetype_to_stroke_dash import (now only used in encoding.rs) Benefits: - Single source of truth for SETTING/defaults precedence logic - All conversions (size, linewidth, linetype) go through same code path - Reduced code: ~40 lines → ~28 lines - Prepares for future writers (ggplot2, plotters) to reuse precedence logic Co-Authored-By: Claude Sonnet 4.5 --- src/writer/vegalite/mod.rs | 65 ++++++++++++++------------------------ 1 file changed, 24 insertions(+), 41 deletions(-) diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 8845f147..1349581c 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -28,7 +28,6 @@ mod layer; // ArrayElement is used in tests and for pattern matching; suppress unused import warning #[allow(unused_imports)] use crate::plot::ArrayElement; -use crate::plot::scale::linetype_to_stroke_dash; use crate::plot::ParameterValue; use crate::writer::Writer; use crate::{ @@ -250,54 +249,38 @@ fn build_layer_encoding( } } - // Add aesthetic parameters from SETTING as literal encodings - // (e.g., SETTING color => 'red' becomes {"color": {"value": "red"}}) - // Only parameters that are supported aesthetics for this geom type are included - let supported_aesthetics = layer.geom.aesthetics().supported(); - for (param_name, param_value) in &layer.parameters { - if supported_aesthetics.contains(¶m_name.as_str()) { - let channel_name = map_aesthetic_name(param_name); - // Only add if not already set by MAPPING (MAPPING takes precedence) - if !encoding.contains_key(&channel_name) { - // Convert size, linewidth, and linetype to Vega-Lite formats - let converted_value = match (param_name.as_str(), param_value) { - // Size: interpret as radius in points, convert to area in pixels^2 - ("size", ParameterValue::Number(n)) => json!(n * n * POINTS_TO_AREA), - // Linewidth: interpret as width in points, convert to pixels - ("linewidth", ParameterValue::Number(n)) => json!(n * POINTS_TO_PIXELS), - // Linetype: convert string patterns to strokeDash arrays - ("linetype", ParameterValue::String(s)) => { - linetype_to_stroke_dash(s).map(|arr| json!(arr)).unwrap_or_else(|| json!(s)) - } - // Other aesthetics: pass through unchanged - _ => param_value.to_json(), - }; - encoding.insert(channel_name, json!({"value": converted_value})); - } - } - } - - // Add default aesthetic values from geom (lowest priority) - // Only apply if not already set by MAPPING or SETTING + // Add aesthetic values from SETTING (higher priority) and defaults (lower priority) + // Precedence order: MAPPING > SETTING > defaults + // SETTING and defaults are mutually exclusive, so process both in a unified loop let geom_aesthetics = layer.geom.aesthetics(); - for (aesthetic_name, default_value) in geom_aesthetics.defaults { + let supported_aesthetics = geom_aesthetics.supported(); + + for aesthetic_name in supported_aesthetics { let channel_name = map_aesthetic_name(aesthetic_name); - // Skip if already set by MAPPING or SETTING + // Skip if already set by MAPPING (highest precedence) if encoding.contains_key(&channel_name) { continue; } - // Convert to AestheticValue - let aesthetic_value = default_value.to_aesthetic_value(); - - // Skip if not a literal or if it's Null (Required/Null/Delayed all become Null) - match aesthetic_value { - AestheticValue::Literal(ParameterValue::Null) | AestheticValue::Column { .. } => continue, - AestheticValue::Literal(_) => {} // Has a real value, proceed - } + // Try SETTING parameter first, then default value + let aesthetic_value = if let Some(param_value) = layer.parameters.get(aesthetic_name) { + // SETTING parameter (literal value) + AestheticValue::Literal(param_value.clone()) + } else if let Some(default_value) = geom_aesthetics.get(aesthetic_name) { + // Default from geom definition + let value = default_value.to_aesthetic_value(); + // Skip if Null or Column reference (only apply literal defaults) + match value { + AestheticValue::Literal(ParameterValue::Null) | AestheticValue::Column { .. } => continue, + _ => value, + } + } else { + // No SETTING and no default for this aesthetic + continue; + }; - // Use existing encoding builder with unit conversions + // Build encoding channel with conversions (size, linewidth, linetype) let channel_encoding = build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; encoding.insert(channel_name, channel_encoding); } From b1d7999429bbb0a4203a7aa0462d7dbe755fc172 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 17:16:55 +0100 Subject: [PATCH 08/19] Centralize SETTING/defaults precedence logic in Layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Layer::get_aesthetic_value() to centralize SETTING > defaults precedence logic, preparing for multiple writers (ggplot2, plotters). Changes: - Add DefaultAestheticValue::to_parameter_value() to extract literal values (returns ParameterValue, not Option - uses Null for non-literals) - Refactor to_aesthetic_value() to reuse to_parameter_value() (eliminates duplication) - Add Layer::get_aesthetic_value() for SETTING > defaults resolution - Update Vega-Lite writer to use get_aesthetic_value() (28 lines → 18 lines) Benefits: - Precedence logic centralized in Layer (not duplicated per writer) - Future writers can reuse get_aesthetic_value() for consistent behavior - Conversions (size, linewidth, linetype) remain writer-specific - Cleaner, more maintainable code with simpler signatures Co-Authored-By: Claude Sonnet 4.5 --- src/plot/layer/mod.rs | 25 +++++++++++++++++++++++++ src/plot/types.rs | 20 ++++++++++++++++---- src/writer/vegalite/mod.rs | 34 ++++++++++------------------------ 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index 3a1ec047..aa687172 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -117,6 +117,31 @@ impl Layer { } } + /// Get resolved aesthetic value with precedence: SETTING > defaults + /// + /// Returns a literal ParameterValue if the aesthetic has a value from either: + /// - SETTING parameters (user-specified, highest priority) + /// - Geom defaults (fallback, lower priority) + /// + /// Returns None for defaults that are Required, Null, Delayed, or Column references. + /// Use this in writers to centralize SETTING/defaults precedence logic. + pub fn get_aesthetic_value(&self, aesthetic: &str) -> Option { + // Check SETTING first (user-specified) + if let Some(value) = self.parameters.get(aesthetic) { + return Some(value.clone()); + } + + // Fall back to geom default (filter out Null = non-literal defaults) + if let Some(default_value) = self.geom.aesthetics().get(aesthetic) { + match default_value.to_parameter_value() { + ParameterValue::Null => None, + value => Some(value), + } + } else { + None + } + } + /// Check if this layer has the required aesthetics for its geom pub fn validate_required_aesthetics(&self) -> std::result::Result<(), String> { for aesthetic in self.geom.aesthetics().required() { diff --git a/src/plot/types.rs b/src/plot/types.rs index 8c1b0357..58a85abd 100644 --- a/src/plot/types.rs +++ b/src/plot/types.rs @@ -264,14 +264,26 @@ pub enum DefaultAestheticValue { } impl DefaultAestheticValue { + /// Convert to ParameterValue + /// + /// Returns String/Number/Boolean for literal defaults. + /// Returns Null for Column/Null/Required/Delayed (non-literal variants). + /// Use this to extract SETTING-compatible values from defaults. + pub fn to_parameter_value(&self) -> ParameterValue { + match self { + Self::String(s) => ParameterValue::String(s.to_string()), + Self::Number(n) => ParameterValue::Number(*n), + Self::Boolean(b) => ParameterValue::Boolean(*b), + Self::Column(_) | Self::Null | Self::Required | Self::Delayed => ParameterValue::Null, + } + } + /// Convert to owned AestheticValue pub fn to_aesthetic_value(&self) -> AestheticValue { match self { Self::Column(name) => AestheticValue::standard_column(name.to_string()), - Self::String(s) => AestheticValue::Literal(ParameterValue::String(s.to_string())), - Self::Number(n) => AestheticValue::Literal(ParameterValue::Number(*n)), - Self::Boolean(b) => AestheticValue::Literal(ParameterValue::Boolean(*b)), - Self::Null | Self::Required | Self::Delayed => AestheticValue::Literal(ParameterValue::Null), + // All literal variants (String/Number/Boolean) and non-literals (Null/Required/Delayed) + _ => AestheticValue::Literal(self.to_parameter_value()), } } } diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 1349581c..42ce0dcb 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -28,7 +28,6 @@ mod layer; // ArrayElement is used in tests and for pattern matching; suppress unused import warning #[allow(unused_imports)] use crate::plot::ArrayElement; -use crate::plot::ParameterValue; use crate::writer::Writer; use crate::{ is_primary_positional, naming, primary_aesthetic, AestheticValue, DataFrame, GgsqlError, Plot, @@ -251,9 +250,8 @@ fn build_layer_encoding( // Add aesthetic values from SETTING (higher priority) and defaults (lower priority) // Precedence order: MAPPING > SETTING > defaults - // SETTING and defaults are mutually exclusive, so process both in a unified loop - let geom_aesthetics = layer.geom.aesthetics(); - let supported_aesthetics = geom_aesthetics.supported(); + // Use Layer::get_aesthetic_value() to centralize precedence logic + let supported_aesthetics = layer.geom.aesthetics().supported(); for aesthetic_name in supported_aesthetics { let channel_name = map_aesthetic_name(aesthetic_name); @@ -263,26 +261,14 @@ fn build_layer_encoding( continue; } - // Try SETTING parameter first, then default value - let aesthetic_value = if let Some(param_value) = layer.parameters.get(aesthetic_name) { - // SETTING parameter (literal value) - AestheticValue::Literal(param_value.clone()) - } else if let Some(default_value) = geom_aesthetics.get(aesthetic_name) { - // Default from geom definition - let value = default_value.to_aesthetic_value(); - // Skip if Null or Column reference (only apply literal defaults) - match value { - AestheticValue::Literal(ParameterValue::Null) | AestheticValue::Column { .. } => continue, - _ => value, - } - } else { - // No SETTING and no default for this aesthetic - continue; - }; - - // Build encoding channel with conversions (size, linewidth, linetype) - let channel_encoding = build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; - encoding.insert(channel_name, channel_encoding); + // Get resolved value (SETTING > defaults, only literal values) + if let Some(param_value) = layer.get_aesthetic_value(aesthetic_name) { + let aesthetic_value = AestheticValue::Literal(param_value); + // Build encoding channel with conversions (size, linewidth, linetype) + let channel_encoding = + build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; + encoding.insert(channel_name, channel_encoding); + } } // Add detail encoding for partition_by columns (grouping) From 7dceccd5740939e3cca2b6cc8d861416fdcb5822 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Thu, 19 Feb 2026 17:31:05 +0100 Subject: [PATCH 09/19] remove dangling test & cargo fmt --- src/plot/layer/geom/mod.rs | 2 +- src/plot/layer/geom/types.rs | 2 +- src/plot/layer/mod.rs | 2 +- src/plot/main.rs | 1 - src/writer/vegalite/encoding.rs | 4 +++- src/writer/vegalite/mod.rs | 9 ++++----- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index eee995a0..38aa90ca 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -51,7 +51,7 @@ mod violin; mod vline; // Re-export types -pub use types::{DefaultParam, DefaultParamValue, DefaultAesthetics, StatResult}; +pub use types::{DefaultAesthetics, DefaultParam, DefaultParamValue, StatResult}; // Re-export aesthetic family utilities from the central module pub use crate::plot::aesthetic::{get_aesthetic_family, AESTHETIC_FAMILIES}; diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index 804cc920..178209cf 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -2,7 +2,7 @@ //! //! These types are used by all geom implementations and are shared across the module. -use crate::{Mappings, plot::types::DefaultAestheticValue}; +use crate::{plot::types::DefaultAestheticValue, Mappings}; /// Default aesthetic values for a geom type /// diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index aa687172..f9e35a5b 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -11,7 +11,7 @@ pub mod geom; // Re-export geom types for convenience pub use geom::{ - DefaultParam, DefaultParamValue, Geom, DefaultAesthetics, GeomTrait, GeomType, StatResult, + DefaultAesthetics, DefaultParam, DefaultParamValue, Geom, GeomTrait, GeomType, StatResult, }; use crate::plot::types::{AestheticValue, DataSource, Mappings, ParameterValue, SqlExpression}; diff --git a/src/plot/main.rs b/src/plot/main.rs index 4f3bdfde..473c7619 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -397,7 +397,6 @@ mod tests { // Bar geom - optional x and y (stat decides aggregation) let bar = Geom::bar().aesthetics(); assert!(bar.is_supported("fill")); - assert!(bar.is_supported("width")); assert!(bar.is_supported("y")); // Bar accepts optional y assert!(bar.is_supported("x")); // Bar accepts optional x assert_eq!(bar.required(), &[] as &[&str]); // No required aesthetics diff --git a/src/writer/vegalite/encoding.rs b/src/writer/vegalite/encoding.rs index ea0124e7..73d497cc 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -859,7 +859,9 @@ fn build_column_encoding( fn build_literal_encoding(aesthetic: &str, lit: &ParameterValue) -> Result { let val = match lit { ParameterValue::String(s) => match aesthetic { - "linetype" => linetype_to_stroke_dash(s).map(|arr| json!(arr)).unwrap_or_else(|| json!(s)), + "linetype" => linetype_to_stroke_dash(s) + .map(|arr| json!(arr)) + .unwrap_or_else(|| json!(s)), _ => json!(s), }, ParameterValue::Number(n) => { diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 42ce0dcb..e4aabf38 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -1206,11 +1206,10 @@ mod tests { // Line with linetype as SETTING (literal) let mut spec = Plot::new(); - let layer = build_layer(Geom::line()) - .with_aesthetic( - "linetype".to_string(), - AestheticValue::Literal(ParameterValue::String("dashed".to_string())), - ); + let layer = build_layer(Geom::line()).with_aesthetic( + "linetype".to_string(), + AestheticValue::Literal(ParameterValue::String("dashed".to_string())), + ); spec.layers.push(layer); let result = writer.write(&spec, &wrap_data(simple_df())); From c26407b648740ba020296062ff13daeb4e48c41d Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 20 Feb 2026 09:50:24 +0100 Subject: [PATCH 10/19] It turns out the unreachable statement did in fact become reachable --- src/writer/vegalite/encoding.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/writer/vegalite/encoding.rs b/src/writer/vegalite/encoding.rs index 73d497cc..aa3a3fb8 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -873,10 +873,7 @@ fn build_literal_encoding(aesthetic: &str, lit: &ParameterValue) -> Result json!(n), } } - ParameterValue::Boolean(b) => json!(b), - ParameterValue::Array(_) | ParameterValue::Null => { - unreachable!("Grammar prevents arrays and null in literal aesthetic mappings") - } + _ => lit.to_json(), }; Ok(json!({"value": val})) } From 64823358866013c7b110e79e8e6579dabb891e12 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 11:55:41 +0100 Subject: [PATCH 11/19] review area/density opacity --- src/plot/layer/geom/area.rs | 2 +- src/plot/layer/geom/density.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plot/layer/geom/area.rs b/src/plot/layer/geom/area.rs index 5df42c4c..7d30b62c 100644 --- a/src/plot/layer/geom/area.rs +++ b/src/plot/layer/geom/area.rs @@ -18,7 +18,7 @@ impl GeomTrait for Area { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("black")), + ("fill", DefaultAestheticValue::String("#000000B2")), ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index 30f3afde..49f65b36 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -29,8 +29,8 @@ impl GeomTrait for Density { defaults: &[ ("x", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("black")), - ("stroke", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("#000000B2")), + ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), ("linetype", DefaultAestheticValue::String("solid")), From 580312122e247d4bea8e16a9111d360ff5f2b099 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 12:28:17 +0100 Subject: [PATCH 12/19] Move aesthetic resolution from writer to execution phase Aesthetic harmonization (MAPPING > SETTING > defaults) now happens once during execution rather than per-writer. This creates a single source of truth for aesthetic values. Changes: - Add resolved_aesthetics field to Layer (HashMap of resolved values) - Add Layer::resolve_aesthetics() method called during execution - Remove Layer::get_aesthetic_value() (logic now inlined) - Simplify Vega-Lite writer to use resolved_aesthetics directly - Add comprehensive tests for resolution logic This ensures all writers handle aesthetics consistently and reduces duplication across writer implementations. Co-Authored-By: Claude Sonnet 4.5 --- src/execute/mod.rs | 3 + src/plot/layer/mod.rs | 128 +++++++++++++++++++++++++++++-------- src/writer/vegalite/mod.rs | 48 ++++++++------ 3 files changed, 135 insertions(+), 44 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index fd872a21..3f80fda0 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -658,6 +658,9 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result, + /// Resolved aesthetic values for aesthetics not in MAPPING. + /// Contains values from SETTING and geom defaults (SETTING > defaults). + /// Computed during execution by `compute_resolved_aesthetics()`. + /// Writers use this as single source of truth for non-mapped aesthetics. + pub resolved_aesthetics: HashMap, /// Optional data source for this layer (from MAPPING ... FROM) pub source: Option, /// Optional filter expression for this layer (from FILTER clause) @@ -51,6 +56,7 @@ impl Layer { mappings: Mappings::new(), remappings: Mappings::new(), parameters: HashMap::new(), + resolved_aesthetics: HashMap::new(), source: None, filter: None, order_by: None, @@ -117,31 +123,6 @@ impl Layer { } } - /// Get resolved aesthetic value with precedence: SETTING > defaults - /// - /// Returns a literal ParameterValue if the aesthetic has a value from either: - /// - SETTING parameters (user-specified, highest priority) - /// - Geom defaults (fallback, lower priority) - /// - /// Returns None for defaults that are Required, Null, Delayed, or Column references. - /// Use this in writers to centralize SETTING/defaults precedence logic. - pub fn get_aesthetic_value(&self, aesthetic: &str) -> Option { - // Check SETTING first (user-specified) - if let Some(value) = self.parameters.get(aesthetic) { - return Some(value.clone()); - } - - // Fall back to geom default (filter out Null = non-literal defaults) - if let Some(default_value) = self.geom.aesthetics().get(aesthetic) { - match default_value.to_parameter_value() { - ParameterValue::Null => None, - value => Some(value), - } - } else { - None - } - } - /// Check if this layer has the required aesthetics for its geom pub fn validate_required_aesthetics(&self) -> std::result::Result<(), String> { for aesthetic in self.geom.aesthetics().required() { @@ -173,6 +154,45 @@ impl Layer { } } + /// Resolve aesthetics for all supported aesthetics not in MAPPING. + /// + /// For each supported aesthetic that's not already mapped in MAPPING: + /// - Check SETTING parameters first (user-specified, highest priority) and consume from parameters + /// - Fall back to geom defaults (lower priority) + /// - Store in resolved_aesthetics for writers to use + /// + /// Precedence: MAPPING > SETTING > geom defaults + /// + /// Call this during execution to provide a single source of truth for writers. + pub fn resolve_aesthetics(&mut self) { + self.resolved_aesthetics.clear(); + + let supported_aesthetics = self.geom.aesthetics().supported(); + + for aesthetic_name in supported_aesthetics { + // Skip if already in MAPPING (highest precedence) + if self.mappings.contains_key(aesthetic_name) { + continue; + } + + // Check SETTING first (user-specified) and consume from parameters + if let Some(value) = self.parameters.remove(aesthetic_name) { + self.resolved_aesthetics.insert(aesthetic_name.to_string(), value); + continue; + } + + // Fall back to geom default (filter out Null = non-literal defaults) + if let Some(default_value) = self.geom.aesthetics().get(aesthetic_name) { + match default_value.to_parameter_value() { + ParameterValue::Null => continue, + value => { + self.resolved_aesthetics.insert(aesthetic_name.to_string(), value); + } + } + } + } + } + /// Validate that all SETTING parameters are valid for this layer's geom pub fn validate_settings(&self) -> std::result::Result<(), String> { let valid = self.geom.valid_settings(); @@ -275,3 +295,61 @@ impl Layer { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_resolve_aesthetics_from_settings() { + // Test that resolve_aesthetics() moves aesthetic values from parameters to resolved_aesthetics + let mut layer = Layer::new(Geom::point()); + layer.parameters.insert("size".to_string(), ParameterValue::Number(5.0)); + layer.parameters.insert("opacity".to_string(), ParameterValue::Number(0.8)); + + layer.resolve_aesthetics(); + + // Values should be moved from parameters to resolved_aesthetics + assert!(!layer.parameters.contains_key("size")); + assert!(!layer.parameters.contains_key("opacity")); + assert_eq!(layer.resolved_aesthetics.get("size"), Some(&ParameterValue::Number(5.0))); + assert_eq!(layer.resolved_aesthetics.get("opacity"), Some(&ParameterValue::Number(0.8))); + } + + #[test] + fn test_resolve_aesthetics_from_defaults() { + // Test that resolve_aesthetics() includes geom default values + let mut layer = Layer::new(Geom::point()); + + layer.resolve_aesthetics(); + + // Point geom has default shape = 'circle' + assert_eq!(layer.resolved_aesthetics.get("shape"), Some(&ParameterValue::String("circle".to_string()))); + } + + #[test] + fn test_resolve_aesthetics_skips_mapped() { + // Test that resolve_aesthetics() skips aesthetics that are already in MAPPING + let mut layer = Layer::new(Geom::point()); + layer.mappings.insert("size", AestheticValue::standard_column("my_size".to_string())); + layer.parameters.insert("size".to_string(), ParameterValue::Number(5.0)); + + layer.resolve_aesthetics(); + + // size should stay in parameters (not moved to resolved_aesthetics) because it's in MAPPING + assert!(layer.parameters.contains_key("size")); + assert!(!layer.resolved_aesthetics.contains_key("size")); + } + + #[test] + fn test_resolve_aesthetics_precedence() { + // Test that SETTING takes precedence over geom defaults + let mut layer = Layer::new(Geom::point()); + layer.parameters.insert("shape".to_string(), ParameterValue::String("square".to_string())); + + layer.resolve_aesthetics(); + + // Should use SETTING value, not default + assert_eq!(layer.resolved_aesthetics.get("shape"), Some(&ParameterValue::String("square".to_string()))); + } +} diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index e4aabf38..58f34dcf 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -248,12 +248,10 @@ fn build_layer_encoding( } } - // Add aesthetic values from SETTING (higher priority) and defaults (lower priority) - // Precedence order: MAPPING > SETTING > defaults - // Use Layer::get_aesthetic_value() to centralize precedence logic - let supported_aesthetics = layer.geom.aesthetics().supported(); - - for aesthetic_name in supported_aesthetics { + // Add resolved aesthetic values (from SETTING or geom defaults) + // These were computed during execution in Layer::resolve_aesthetics() + // Precedence order: MAPPING > SETTING > defaults (already resolved) + for (aesthetic_name, param_value) in &layer.resolved_aesthetics { let channel_name = map_aesthetic_name(aesthetic_name); // Skip if already set by MAPPING (highest precedence) @@ -261,14 +259,11 @@ fn build_layer_encoding( continue; } - // Get resolved value (SETTING > defaults, only literal values) - if let Some(param_value) = layer.get_aesthetic_value(aesthetic_name) { - let aesthetic_value = AestheticValue::Literal(param_value); - // Build encoding channel with conversions (size, linewidth, linetype) - let channel_encoding = - build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; - encoding.insert(channel_name, channel_encoding); - } + let aesthetic_value = AestheticValue::Literal(param_value.clone()); + // Build encoding channel with conversions (size, linewidth, linetype) + let channel_encoding = + build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; + encoding.insert(channel_name, channel_encoding); } // Add detail encoding for partition_by columns (grouping) @@ -594,7 +589,10 @@ mod tests { /// ``` fn build_spec(geom: Geom) -> Plot { let mut spec = Plot::new(); - spec.layers.push(build_layer(geom)); + let mut layer = build_layer(geom); + // Resolve aesthetics (normally done in execution pipeline) + layer.resolve_aesthetics(); + spec.layers.push(layer); spec } @@ -1112,8 +1110,11 @@ mod tests { // Point with SETTING opacity => 0.5 should override default (1.0) let mut spec = Plot::new(); - let layer = build_layer(Geom::point()) + let mut layer = build_layer(Geom::point()) .with_parameter("opacity".to_string(), ParameterValue::Number(0.5)); + + // Resolve aesthetics (normally done in execution pipeline) + layer.resolve_aesthetics(); spec.layers.push(layer); let result = writer.write(&spec, &wrap_data(simple_df())); @@ -1133,7 +1134,7 @@ mod tests { // Point with MAPPING stroke AS stroke should override default let mut spec = Plot::new(); - let layer = Layer::new(Geom::point()) + let mut layer = Layer::new(Geom::point()) .with_aesthetic( "x".to_string(), AestheticValue::standard_column("x".to_string()), @@ -1146,6 +1147,9 @@ mod tests { "stroke".to_string(), AestheticValue::standard_column("stroke".to_string()), ); + + // Resolve aesthetics (normally done in execution pipeline) + layer.resolve_aesthetics(); spec.layers.push(layer); let df = df! { @@ -1174,7 +1178,7 @@ mod tests { // Point has linetype as Null - should not appear in encoding let mut spec = Plot::new(); - let layer = Layer::new(Geom::point()) + let mut layer = Layer::new(Geom::point()) .with_aesthetic( "x".to_string(), AestheticValue::standard_column("x".to_string()), @@ -1183,6 +1187,9 @@ mod tests { "y".to_string(), AestheticValue::standard_column("y".to_string()), ); + + // Resolve aesthetics (normally done in execution pipeline) + layer.resolve_aesthetics(); spec.layers.push(layer); let df = df! { @@ -1206,10 +1213,13 @@ mod tests { // Line with linetype as SETTING (literal) let mut spec = Plot::new(); - let layer = build_layer(Geom::line()).with_aesthetic( + let mut layer = build_layer(Geom::line()).with_aesthetic( "linetype".to_string(), AestheticValue::Literal(ParameterValue::String("dashed".to_string())), ); + + // Resolve aesthetics (normally done in execution pipeline) + layer.resolve_aesthetics(); spec.layers.push(layer); let result = writer.write(&spec, &wrap_data(simple_df())); From 8ca6d529e105d08dd571bfa9ca96dd4ed364eb80 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 12:55:30 +0100 Subject: [PATCH 13/19] Merge resolved_aesthetics into mappings for single source of truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate aesthetic resolution by removing the separate resolved_aesthetics field and inserting SETTING/defaults directly into mappings as Literal values. The key insight is timing: resolve_aesthetics() now runs AFTER query literals have been converted to columns, ensuring proper distinction. Changes: - Remove resolved_aesthetics field from Layer struct - Update resolve_aesthetics() to insert into mappings as AestheticValue::Literal - Move resolve_aesthetics() call to after remappings (line 733) - Simplify Vega-Lite writer to single loop over mappings - Update all tests to check mappings instead of resolved_aesthetics - Add comprehensive documentation explaining query literal vs SETTING/default distinction Flow: 1. Query literals ('foo' AS color) → parsed as Literal in mappings 2. build_layer_select_list() → converts to SQL columns 3. update_mappings_for_aesthetic_columns() → Literal → Column in mappings 4. resolve_aesthetics() → adds SETTING/defaults as NEW Literals (key change) 5. Writer → Column gets scales, Literal renders as constant value This ensures: - Query literals can have scales applied (they're columns) - SETTING/defaults remain constant values (they're Literals) - Single source of truth for all aesthetic values - Correct precedence: MAPPING > REMAPPING > SETTING > defaults Co-Authored-By: Claude Sonnet 4.5 --- src/execute/mod.rs | 8 +++-- src/plot/layer/mod.rs | 71 +++++++++++++++++++++++++++----------- src/writer/vegalite/mod.rs | 22 +++--------- 3 files changed, 60 insertions(+), 41 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 3f80fda0..a232596d 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -658,9 +658,6 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result(query: &str, reader: &R) -> Result, - /// Resolved aesthetic values for aesthetics not in MAPPING. - /// Contains values from SETTING and geom defaults (SETTING > defaults). - /// Computed during execution by `compute_resolved_aesthetics()`. - /// Writers use this as single source of truth for non-mapped aesthetics. - pub resolved_aesthetics: HashMap, /// Optional data source for this layer (from MAPPING ... FROM) pub source: Option, /// Optional filter expression for this layer (from FILTER clause) @@ -56,7 +70,6 @@ impl Layer { mappings: Mappings::new(), remappings: Mappings::new(), parameters: HashMap::new(), - resolved_aesthetics: HashMap::new(), source: None, filter: None, order_by: None, @@ -159,14 +172,16 @@ impl Layer { /// For each supported aesthetic that's not already mapped in MAPPING: /// - Check SETTING parameters first (user-specified, highest priority) and consume from parameters /// - Fall back to geom defaults (lower priority) - /// - Store in resolved_aesthetics for writers to use + /// - Insert into mappings as `AestheticValue::Literal` /// /// Precedence: MAPPING > SETTING > geom defaults /// + /// **Important**: Query literals from MAPPING (`'foo' AS color`) have already been converted + /// to columns during query execution, so this only adds SETTING/default literals which + /// remain as `AestheticValue::Literal` and render without scale transformations. + /// /// Call this during execution to provide a single source of truth for writers. pub fn resolve_aesthetics(&mut self) { - self.resolved_aesthetics.clear(); - let supported_aesthetics = self.geom.aesthetics().supported(); for aesthetic_name in supported_aesthetics { @@ -177,7 +192,7 @@ impl Layer { // Check SETTING first (user-specified) and consume from parameters if let Some(value) = self.parameters.remove(aesthetic_name) { - self.resolved_aesthetics.insert(aesthetic_name.to_string(), value); + self.mappings.insert(aesthetic_name, AestheticValue::Literal(value)); continue; } @@ -186,7 +201,7 @@ impl Layer { match default_value.to_parameter_value() { ParameterValue::Null => continue, value => { - self.resolved_aesthetics.insert(aesthetic_name.to_string(), value); + self.mappings.insert(aesthetic_name, AestheticValue::Literal(value)); } } } @@ -302,18 +317,24 @@ mod tests { #[test] fn test_resolve_aesthetics_from_settings() { - // Test that resolve_aesthetics() moves aesthetic values from parameters to resolved_aesthetics + // Test that resolve_aesthetics() moves aesthetic values from parameters to mappings let mut layer = Layer::new(Geom::point()); layer.parameters.insert("size".to_string(), ParameterValue::Number(5.0)); layer.parameters.insert("opacity".to_string(), ParameterValue::Number(0.8)); layer.resolve_aesthetics(); - // Values should be moved from parameters to resolved_aesthetics + // Values should be moved from parameters to mappings as Literal assert!(!layer.parameters.contains_key("size")); assert!(!layer.parameters.contains_key("opacity")); - assert_eq!(layer.resolved_aesthetics.get("size"), Some(&ParameterValue::Number(5.0))); - assert_eq!(layer.resolved_aesthetics.get("opacity"), Some(&ParameterValue::Number(0.8))); + assert_eq!( + layer.mappings.get("size"), + Some(&AestheticValue::Literal(ParameterValue::Number(5.0))) + ); + assert_eq!( + layer.mappings.get("opacity"), + Some(&AestheticValue::Literal(ParameterValue::Number(0.8))) + ); } #[test] @@ -324,7 +345,10 @@ mod tests { layer.resolve_aesthetics(); // Point geom has default shape = 'circle' - assert_eq!(layer.resolved_aesthetics.get("shape"), Some(&ParameterValue::String("circle".to_string()))); + assert_eq!( + layer.mappings.get("shape"), + Some(&AestheticValue::Literal(ParameterValue::String("circle".to_string()))) + ); } #[test] @@ -336,9 +360,13 @@ mod tests { layer.resolve_aesthetics(); - // size should stay in parameters (not moved to resolved_aesthetics) because it's in MAPPING + // size should stay in parameters (not moved to mappings) because it's already in MAPPING assert!(layer.parameters.contains_key("size")); - assert!(!layer.resolved_aesthetics.contains_key("size")); + // The mapping should still be the Column, not replaced with Literal + assert!(matches!( + layer.mappings.get("size"), + Some(AestheticValue::Column { .. }) + )); } #[test] @@ -350,6 +378,9 @@ mod tests { layer.resolve_aesthetics(); // Should use SETTING value, not default - assert_eq!(layer.resolved_aesthetics.get("shape"), Some(&ParameterValue::String("square".to_string()))); + assert_eq!( + layer.mappings.get("shape"), + Some(&AestheticValue::Literal(ParameterValue::String("square".to_string()))) + ); } } diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 58f34dcf..8ad24718 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -231,6 +231,10 @@ fn build_layer_encoding( }; // Build encoding channels for each aesthetic mapping + // Mappings contains: + // 1. Column references from MAPPING clause (apply scales) + // 2. Query literals from MAPPING (converted to columns, apply scales) + // 3. Literals from SETTING/defaults (remain as literals, no scales) for (aesthetic, value) in &layer.mappings.aesthetics { let channel_name = map_aesthetic_name(aesthetic); let channel_encoding = build_encoding_channel(aesthetic, value, &mut enc_ctx)?; @@ -248,24 +252,6 @@ fn build_layer_encoding( } } - // Add resolved aesthetic values (from SETTING or geom defaults) - // These were computed during execution in Layer::resolve_aesthetics() - // Precedence order: MAPPING > SETTING > defaults (already resolved) - for (aesthetic_name, param_value) in &layer.resolved_aesthetics { - let channel_name = map_aesthetic_name(aesthetic_name); - - // Skip if already set by MAPPING (highest precedence) - if encoding.contains_key(&channel_name) { - continue; - } - - let aesthetic_value = AestheticValue::Literal(param_value.clone()); - // Build encoding channel with conversions (size, linewidth, linetype) - let channel_encoding = - build_encoding_channel(aesthetic_name, &aesthetic_value, &mut enc_ctx)?; - encoding.insert(channel_name, channel_encoding); - } - // Add detail encoding for partition_by columns (grouping) if let Some(detail) = build_detail_encoding(&layer.partition_by) { encoding.insert("detail".to_string(), detail); From 05ee9c0398abe06417854c40980c239882fd5594 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 13:57:03 +0100 Subject: [PATCH 14/19] Fix broken test expectation --- src/reader/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/reader/mod.rs b/src/reader/mod.rs index 5565668c..b99c6ceb 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -340,7 +340,8 @@ mod tests { let metadata = spec.metadata(); assert_eq!(metadata.rows, 3); - assert_eq!(metadata.columns.len(), 2); + // Columns now includes both user mappings (x, y) and resolved defaults (size, stroke, fill, opacity, shape, linewidth) + assert_eq!(metadata.columns.len(), 8); assert!(metadata.columns.contains(&"x".to_string())); assert!(metadata.columns.contains(&"y".to_string())); assert_eq!(metadata.layer_count, 1); From f5cbb180681d39ea4cfb5dbc576de4375398b583 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 14:12:19 +0100 Subject: [PATCH 15/19] cargo fmt --- src/plot/layer/mod.rs | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index 5d5724e0..84076071 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -192,7 +192,8 @@ impl Layer { // Check SETTING first (user-specified) and consume from parameters if let Some(value) = self.parameters.remove(aesthetic_name) { - self.mappings.insert(aesthetic_name, AestheticValue::Literal(value)); + self.mappings + .insert(aesthetic_name, AestheticValue::Literal(value)); continue; } @@ -201,7 +202,8 @@ impl Layer { match default_value.to_parameter_value() { ParameterValue::Null => continue, value => { - self.mappings.insert(aesthetic_name, AestheticValue::Literal(value)); + self.mappings + .insert(aesthetic_name, AestheticValue::Literal(value)); } } } @@ -319,8 +321,12 @@ mod tests { fn test_resolve_aesthetics_from_settings() { // Test that resolve_aesthetics() moves aesthetic values from parameters to mappings let mut layer = Layer::new(Geom::point()); - layer.parameters.insert("size".to_string(), ParameterValue::Number(5.0)); - layer.parameters.insert("opacity".to_string(), ParameterValue::Number(0.8)); + layer + .parameters + .insert("size".to_string(), ParameterValue::Number(5.0)); + layer + .parameters + .insert("opacity".to_string(), ParameterValue::Number(0.8)); layer.resolve_aesthetics(); @@ -347,7 +353,9 @@ mod tests { // Point geom has default shape = 'circle' assert_eq!( layer.mappings.get("shape"), - Some(&AestheticValue::Literal(ParameterValue::String("circle".to_string()))) + Some(&AestheticValue::Literal(ParameterValue::String( + "circle".to_string() + ))) ); } @@ -355,8 +363,13 @@ mod tests { fn test_resolve_aesthetics_skips_mapped() { // Test that resolve_aesthetics() skips aesthetics that are already in MAPPING let mut layer = Layer::new(Geom::point()); - layer.mappings.insert("size", AestheticValue::standard_column("my_size".to_string())); - layer.parameters.insert("size".to_string(), ParameterValue::Number(5.0)); + layer.mappings.insert( + "size", + AestheticValue::standard_column("my_size".to_string()), + ); + layer + .parameters + .insert("size".to_string(), ParameterValue::Number(5.0)); layer.resolve_aesthetics(); @@ -373,14 +386,19 @@ mod tests { fn test_resolve_aesthetics_precedence() { // Test that SETTING takes precedence over geom defaults let mut layer = Layer::new(Geom::point()); - layer.parameters.insert("shape".to_string(), ParameterValue::String("square".to_string())); + layer.parameters.insert( + "shape".to_string(), + ParameterValue::String("square".to_string()), + ); layer.resolve_aesthetics(); // Should use SETTING value, not default assert_eq!( layer.mappings.get("shape"), - Some(&AestheticValue::Literal(ParameterValue::String("square".to_string()))) + Some(&AestheticValue::Literal(ParameterValue::String( + "square".to_string() + ))) ); } } From 5724b50bc5da7eeb60e8863aeed2486190e23087 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 14:56:20 +0100 Subject: [PATCH 16/19] set opaque default fill for area/density --- src/plot/layer/geom/area.rs | 2 +- src/plot/layer/geom/density.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plot/layer/geom/area.rs b/src/plot/layer/geom/area.rs index 7d30b62c..5df42c4c 100644 --- a/src/plot/layer/geom/area.rs +++ b/src/plot/layer/geom/area.rs @@ -18,7 +18,7 @@ impl GeomTrait for Area { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index 49f65b36..2651b9eb 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -29,7 +29,7 @@ impl GeomTrait for Density { defaults: &[ ("x", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), From 91613eed215428f80118a28a04f38afb80ba683f Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 15:30:56 +0100 Subject: [PATCH 17/19] retarget opacity to fillOpacity when fill is supported --- src/writer/vegalite/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 93a46e92..80d911c7 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -254,7 +254,12 @@ fn build_layer_encoding( continue; } - let channel_name = map_aesthetic_name(aesthetic); + let mut channel_name = map_aesthetic_name(aesthetic); + // Opacity is retargeted to the fill when fill is supported + if channel_name == "opacity" && layer.mappings.contains_key("fill") { + channel_name = "fillOpacity".to_string(); + } + let channel_encoding = build_encoding_channel(aesthetic, value, &mut enc_ctx)?; encoding.insert(channel_name, channel_encoding); From b8b4ae83b45bcdc0950e72ccdbf5430c82f3d22d Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 15:34:21 +0100 Subject: [PATCH 18/19] Don't bake in opacity into the fill colour --- src/plot/layer/geom/bar.rs | 4 ++-- src/plot/layer/geom/boxplot.rs | 4 ++-- src/plot/layer/geom/histogram.rs | 4 ++-- src/plot/layer/geom/point.rs | 4 ++-- src/plot/layer/geom/polygon.rs | 4 ++-- src/plot/layer/geom/ribbon.rs | 4 ++-- src/plot/layer/geom/violin.rs | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/plot/layer/geom/bar.rs b/src/plot/layer/geom/bar.rs index 3bbce5f0..763a6774 100644 --- a/src/plot/layer/geom/bar.rs +++ b/src/plot/layer/geom/bar.rs @@ -33,9 +33,9 @@ impl GeomTrait for Bar { ("x", DefaultAestheticValue::Null), // Optional - stat may provide ("y", DefaultAestheticValue::Null), // Optional - stat may compute ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(0.8)), ], } } diff --git a/src/plot/layer/geom/boxplot.rs b/src/plot/layer/geom/boxplot.rs index dbae53ea..e3c513ad 100644 --- a/src/plot/layer/geom/boxplot.rs +++ b/src/plot/layer/geom/boxplot.rs @@ -27,9 +27,9 @@ impl GeomTrait for Boxplot { ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), ("stroke", DefaultAestheticValue::String("black")), - ("fill", DefaultAestheticValue::String("#FFFFFFCC")), + ("fill", DefaultAestheticValue::String("white")), ("linewidth", DefaultAestheticValue::Number(1.0)), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(0.8)), ("linetype", DefaultAestheticValue::String("solid")), ("size", DefaultAestheticValue::Number(3.0)), ("shape", DefaultAestheticValue::String("circle")), diff --git a/src/plot/layer/geom/histogram.rs b/src/plot/layer/geom/histogram.rs index 205ff661..8c3651d2 100644 --- a/src/plot/layer/geom/histogram.rs +++ b/src/plot/layer/geom/histogram.rs @@ -24,9 +24,9 @@ impl GeomTrait for Histogram { defaults: &[ ("x", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(0.8)), // y and xend are produced by stat_histogram but not valid for manual MAPPING ("y", DefaultAestheticValue::Delayed), ("xend", DefaultAestheticValue::Delayed), diff --git a/src/plot/layer/geom/point.rs b/src/plot/layer/geom/point.rs index 9b4b18fd..8ede18a1 100644 --- a/src/plot/layer/geom/point.rs +++ b/src/plot/layer/geom/point.rs @@ -19,8 +19,8 @@ impl GeomTrait for Point { ("y", DefaultAestheticValue::Required), ("size", DefaultAestheticValue::Number(3.0)), ("stroke", DefaultAestheticValue::String("black")), - ("fill", DefaultAestheticValue::String("#000000B2")), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("fill", DefaultAestheticValue::String("black")), + ("opacity", DefaultAestheticValue::Number(0.8)), ("shape", DefaultAestheticValue::String("circle")), ("linewidth", DefaultAestheticValue::Number(1.0)), ], diff --git a/src/plot/layer/geom/polygon.rs b/src/plot/layer/geom/polygon.rs index 50804a68..ad5c1098 100644 --- a/src/plot/layer/geom/polygon.rs +++ b/src/plot/layer/geom/polygon.rs @@ -17,9 +17,9 @@ impl GeomTrait for Polygon { defaults: &[ ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), ("linetype", DefaultAestheticValue::String("solid")), ], diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 76b7b717..6cf81638 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -18,9 +18,9 @@ impl GeomTrait for Ribbon { ("x", DefaultAestheticValue::Required), ("ymin", DefaultAestheticValue::Required), ("ymax", DefaultAestheticValue::Required), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), ("linetype", DefaultAestheticValue::String("solid")), ], diff --git a/src/plot/layer/geom/violin.rs b/src/plot/layer/geom/violin.rs index 3a0e342e..b7872624 100644 --- a/src/plot/layer/geom/violin.rs +++ b/src/plot/layer/geom/violin.rs @@ -25,9 +25,9 @@ impl GeomTrait for Violin { ("x", DefaultAestheticValue::Required), ("y", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), - ("fill", DefaultAestheticValue::String("#000000B2")), + ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), - ("opacity", DefaultAestheticValue::Number(1.0)), + ("opacity", DefaultAestheticValue::Number(0.8)), ("linewidth", DefaultAestheticValue::Number(1.0)), ("linetype", DefaultAestheticValue::String("solid")), ("offset", DefaultAestheticValue::Delayed), // Computed by stat From f5a230643e18fe79b490c3cd85337f170bc777d9 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 24 Feb 2026 16:23:17 +0100 Subject: [PATCH 19/19] update expectations based on new defaults --- src/writer/vegalite/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 80d911c7..09801aa0 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -1614,8 +1614,8 @@ mod tests { // Point default stroke = "black" assert_eq!(encoding["stroke"]["value"], "black"); - // Point default opacity = 1.0 - assert_eq!(encoding["opacity"]["value"], 1.0); + // Point default opacity = 0.8 (retargeted to fillOpacity when fill is present) + assert_eq!(encoding["fillOpacity"]["value"], 0.8); } #[test] @@ -1638,8 +1638,9 @@ mod tests { let encoding = &json["layer"][0]["encoding"]; - // Should use SETTING value (0.5), not default (1.0) - assert_eq!(encoding["opacity"]["value"], 0.5); + // Should use SETTING value (0.5), not default (0.8) + // Opacity is retargeted to fillOpacity when fill is present + assert_eq!(encoding["fillOpacity"]["value"], 0.5); } #[test]