From d3076435557cd8f7d7e55c762bc228c487239225 Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Wed, 18 Feb 2026 11:27:48 +0100 Subject: [PATCH 1/2] Consolidate aesthetic information and remove x2/y2 --- src/execute/mod.rs | 27 ++-- src/execute/scale.rs | 4 +- src/lib.rs | 6 + src/naming.rs | 16 +- src/parser/builder.rs | 31 +--- src/plot/aesthetic.rs | 264 +++++++++++++++++++++++++++++++ src/plot/layer/geom/bar.rs | 2 +- src/plot/layer/geom/boxplot.rs | 8 +- src/plot/layer/geom/histogram.rs | 8 +- src/plot/layer/geom/mod.rs | 8 +- src/plot/layer/geom/types.rs | 55 +------ src/plot/main.rs | 4 +- src/plot/mod.rs | 2 + src/plot/scale/mod.rs | 2 +- src/plot/scale/scale_type/mod.rs | 18 +-- src/writer/vegalite/coord.rs | 30 +--- src/writer/vegalite/data.rs | 4 +- src/writer/vegalite/encoding.rs | 21 +-- src/writer/vegalite/layer.rs | 4 +- src/writer/vegalite/mod.rs | 16 +- 20 files changed, 343 insertions(+), 187 deletions(-) create mode 100644 src/plot/aesthetic.rs diff --git a/src/execute/mod.rs b/src/execute/mod.rs index a7fce754..37fedd78 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -23,6 +23,7 @@ pub use schema::TypeInfo; use crate::naming; use crate::parser; +use crate::plot::aesthetic::ALL_POSITIONAL; use crate::plot::layer::geom::GeomAesthetics; use crate::plot::{AestheticValue, Layer, Scale, ScaleTypeKind, Schema}; use crate::{DataFrame, GgsqlError, Plot, Result}; @@ -285,12 +286,6 @@ fn add_discrete_columns_to_partition_by( layer_schemas: &[Schema], scales: &[Scale], ) { - // Positional aesthetics should NOT be auto-added to grouping. - // Stats that need to group by positional aesthetics (like bar/histogram) - // already handle this themselves via stat_consumed_aesthetics(). - const POSITIONAL_AESTHETICS: &[&str] = - &["x", "y", "xmin", "xmax", "ymin", "ymax", "xend", "yend"]; - // Build a map of aesthetic -> scale for quick lookup let scale_map: HashMap<&str, &Scale> = scales.iter().map(|s| (s.aesthetic.as_str(), s)).collect(); @@ -307,8 +302,10 @@ fn add_discrete_columns_to_partition_by( let consumed_aesthetics = layer.geom.stat_consumed_aesthetics(); for (aesthetic, value) in &layer.mappings.aesthetics { - // Skip positional aesthetics - these should not trigger auto-grouping - if POSITIONAL_AESTHETICS.contains(&aesthetic.as_str()) { + // Skip positional aesthetics - these should not trigger auto-grouping. + // Stats that need to group by positional aesthetics (like bar/histogram) + // already handle this themselves via stat_consumed_aesthetics(). + if ALL_POSITIONAL.iter().any(|s| s == aesthetic) { continue; } @@ -1071,20 +1068,20 @@ mod tests { let result = prepare_data_with_reader(query, &reader).unwrap(); let layer = &result.specs[0].layers[0]; - // Layer should have y2 in mappings (added by default for bar) + // Layer should have yend in mappings (added by default for bar) assert!( - layer.mappings.aesthetics.contains_key("y2"), - "Bar should have y2 mapping for baseline: {:?}", + layer.mappings.aesthetics.contains_key("yend"), + "Bar should have yend mapping for baseline: {:?}", layer.mappings.aesthetics.keys().collect::>() ); - // The DataFrame should have the y2 column with 0 values + // The DataFrame should have the yend column with 0 values let layer_df = result.data.get(&naming::layer_key(0)).unwrap(); - let y2_col = naming::aesthetic_column("y2"); + let yend_col = naming::aesthetic_column("yend"); assert!( - layer_df.column(&y2_col).is_ok(), + layer_df.column(¥d_col).is_ok(), "DataFrame should have '{}' column: {:?}", - y2_col, + yend_col, layer_df.get_column_names_str() ); } diff --git a/src/execute/scale.rs b/src/execute/scale.rs index c5815a6d..1936053c 100644 --- a/src/execute/scale.rs +++ b/src/execute/scale.rs @@ -1254,15 +1254,15 @@ mod tests { assert!(x_family.contains(&"x")); assert!(x_family.contains(&"xmin")); assert!(x_family.contains(&"xmax")); - assert!(x_family.contains(&"x2")); assert!(x_family.contains(&"xend")); + assert_eq!(x_family.len(), 4); let y_family = get_aesthetic_family("y"); assert!(y_family.contains(&"y")); assert!(y_family.contains(&"ymin")); assert!(y_family.contains(&"ymax")); - assert!(y_family.contains(&"y2")); assert!(y_family.contains(&"yend")); + assert_eq!(y_family.len(), 4); // Test non-family aesthetics return just themselves let color_family = get_aesthetic_family("color"); diff --git a/src/lib.rs b/src/lib.rs index 888970af..3a25705c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,6 +57,12 @@ pub use plot::{ AestheticValue, DataSource, Facet, Geom, Layer, Mappings, Plot, Scale, SqlExpression, }; +// Re-export aesthetic classification utilities +pub use plot::aesthetic::{ + get_aesthetic_family, is_aesthetic_name, is_positional_aesthetic, is_primary_positional, + primary_aesthetic, AESTHETIC_FAMILIES, ALL_POSITIONAL, PRIMARY_POSITIONAL, +}; + // Future modules - not yet implemented // #[cfg(feature = "engine")] // pub mod engine; diff --git a/src/naming.rs b/src/naming.rs index be0fe505..c6088174 100644 --- a/src/naming.rs +++ b/src/naming.rs @@ -267,21 +267,21 @@ pub fn is_synthetic_column(name: &str) -> bool { /// Generate bin end column name for a binned column. /// /// Used by the Vega-Lite writer to store the upper bound of a bin -/// when using `bin: "binned"` encoding with x2/y2 channels. +/// when using `bin: "binned"` encoding with xend/yend channels. /// /// If the column is an aesthetic column (e.g., `__ggsql_aes_x__`), returns -/// the corresponding `2` aesthetic (e.g., `__ggsql_aes_x2__`). +/// the corresponding `end` aesthetic (e.g., `__ggsql_aes_xend__`). /// /// # Example /// ``` /// use ggsql::naming; /// assert_eq!(naming::bin_end_column("temperature"), "__ggsql_bin_end_temperature__"); -/// assert_eq!(naming::bin_end_column("__ggsql_aes_x__"), "__ggsql_aes_x2__"); +/// assert_eq!(naming::bin_end_column("__ggsql_aes_x__"), "__ggsql_aes_xend__"); /// ``` pub fn bin_end_column(column: &str) -> String { - // If it's an aesthetic column, use the x2/y2 naming convention + // If it's an aesthetic column, use the xend/yend naming convention if let Some(aesthetic) = extract_aesthetic_name(column) { - return aesthetic_column(&format!("{}2", aesthetic)); + return aesthetic_column(&format!("{}end", aesthetic)); } format!("{}bin_end_{}{}", GGSQL_PREFIX, column, GGSQL_SUFFIX) } @@ -434,9 +434,9 @@ mod tests { assert_eq!(bin_end_column("x"), "__ggsql_bin_end_x__"); assert_eq!(bin_end_column("value"), "__ggsql_bin_end_value__"); - // Aesthetic columns use the x2/y2 convention - assert_eq!(bin_end_column("__ggsql_aes_x__"), "__ggsql_aes_x2__"); - assert_eq!(bin_end_column("__ggsql_aes_y__"), "__ggsql_aes_y2__"); + // Aesthetic columns use the xend/yend convention + assert_eq!(bin_end_column("__ggsql_aes_x__"), "__ggsql_aes_xend__"); + assert_eq!(bin_end_column("__ggsql_aes_y__"), "__ggsql_aes_yend__"); } #[test] diff --git a/src/parser/builder.rs b/src/parser/builder.rs index da5c0d00..0a07645b 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -3,6 +3,7 @@ //! Takes a tree-sitter parse tree and builds a typed Plot, //! handling all the node types defined in the grammar. +use crate::plot::aesthetic::is_aesthetic_name; use crate::plot::layer::geom::Geom; use crate::plot::scale::{color_to_hex, is_color_aesthetic, Transform}; use crate::plot::*; @@ -972,36 +973,6 @@ fn validate_coord_properties( Ok(()) } -/// Check if a property name is an aesthetic name -fn is_aesthetic_name(name: &str) -> bool { - matches!( - name, - "x" | "y" - | "xmin" - | "xmax" - | "ymin" - | "ymax" - | "xend" - | "yend" - | "color" - | "colour" - | "fill" - | "stroke" - | "opacity" - | "size" - | "shape" - | "linetype" - | "linewidth" - | "width" - | "height" - | "label" - | "family" - | "fontface" - | "hjust" - | "vjust" - ) -} - /// Parse coord type from a coord_type node fn parse_coord_type(node: &Node, source: &SourceTree) -> Result { let text = source.get_text(node); diff --git a/src/plot/aesthetic.rs b/src/plot/aesthetic.rs new file mode 100644 index 00000000..adfd446b --- /dev/null +++ b/src/plot/aesthetic.rs @@ -0,0 +1,264 @@ +//! Aesthetic classification and validation utilities +//! +//! This module provides centralized functions and constants for working with +//! aesthetic names in ggsql. Aesthetics are visual properties that can be mapped +//! to data columns or set to literal values. +//! +//! # Positional vs Legend Aesthetics +//! +//! Aesthetics fall into two categories: +//! - **Positional**: Map to axes (x, y, and variants like xmin, xmax, etc.) +//! - **Legend**: Map to visual properties shown in legends (color, size, shape, etc.) +//! +//! # Aesthetic Families +//! +//! Some aesthetics belong to "families" where variants map to a primary aesthetic. +//! For example, `xmin`, `xmax`, and `xend` all belong to the "x" family. +//! This is used for scale resolution and label computation. + +use std::sync::LazyLock; + +/// Primary positional aesthetics (x and y only) +pub const PRIMARY_POSITIONAL: &[&str] = &["x", "y"]; + +/// Suffixes that create positional variants (e.g., x + "min" = "xmin") +pub const POSITIONAL_SUFFIXES: &[&str] = &["", "min", "max", "end"]; + +/// Maps variant aesthetics to their primary aesthetic family. +/// +/// For example, `xmin`, `xmax`, and `xend` all belong to the "x" family. +/// When computing labels, all family members can contribute to the primary aesthetic's label, +/// with the first aesthetic encountered in a family setting the label. +pub const AESTHETIC_FAMILIES: &[(&str, &str)] = &[ + ("xmin", "x"), + ("xmax", "x"), + ("xend", "x"), + ("ymin", "y"), + ("ymax", "y"), + ("yend", "y"), +]; + +/// All positional aesthetics (primary + variants), computed from PRIMARY_POSITIONAL x POSITIONAL_SUFFIXES +pub static ALL_POSITIONAL: LazyLock> = LazyLock::new(|| { + PRIMARY_POSITIONAL + .iter() + .flat_map(|&base| { + POSITIONAL_SUFFIXES + .iter() + .map(move |&suffix| format!("{}{}", base, suffix)) + }) + .collect() +}); + +/// Check if aesthetic is primary positional (x or y only) +#[inline] +pub fn is_primary_positional(aesthetic: &str) -> bool { + PRIMARY_POSITIONAL.contains(&aesthetic) +} + +/// Check if aesthetic is positional (maps to axis, not legend) +/// +/// Positional aesthetics include x, y, and their variants (xmin, xmax, ymin, ymax, xend, yend). +/// These aesthetics map to axis positions rather than legend entries. +#[inline] +pub fn is_positional_aesthetic(aesthetic: &str) -> bool { + ALL_POSITIONAL.iter().any(|s| s == aesthetic) +} + +/// Check if name is a recognized aesthetic +/// +/// This includes all positional aesthetics plus visual aesthetics like color, size, shape, etc. +#[inline] +pub fn is_aesthetic_name(name: &str) -> bool { + matches!( + name, + "x" | "y" + | "xmin" + | "xmax" + | "ymin" + | "ymax" + | "xend" + | "yend" + | "color" + | "colour" + | "fill" + | "stroke" + | "opacity" + | "size" + | "shape" + | "linetype" + | "linewidth" + | "width" + | "height" + | "label" + | "family" + | "fontface" + | "hjust" + | "vjust" + ) +} + +/// Get the primary aesthetic for a given aesthetic name. +/// +/// Returns the primary family aesthetic if the input is a variant (e.g., "xmin" -> "x"), +/// or returns the aesthetic itself if it's already primary (e.g., "x" -> "x", "fill" -> "fill"). +#[inline] +pub fn primary_aesthetic(aesthetic: &str) -> &str { + AESTHETIC_FAMILIES + .iter() + .find(|(variant, _)| *variant == aesthetic) + .map(|(_, primary)| *primary) + .unwrap_or(aesthetic) +} + +/// Get all aesthetics in the same family as the given aesthetic. +/// +/// For primary aesthetics like "x", returns all family members: `["x", "xmin", "xmax", "x2", "xend"]`. +/// For variant aesthetics like "xmin", returns just `["xmin"]` since scales should be +/// defined for primary aesthetics. +/// For non-family aesthetics like "color", returns just `["color"]`. +/// +/// This is used by scale resolution to find all columns that contribute to a scale's +/// input range (e.g., both `ymin` and `ymax` columns contribute to the "y" scale). +pub fn get_aesthetic_family(aesthetic: &str) -> Vec<&str> { + // First, determine the primary aesthetic + let primary = primary_aesthetic(aesthetic); + + // If aesthetic is not a primary (it's a variant), just return the aesthetic itself + // since scales should be defined for primary aesthetics + if primary != aesthetic { + return vec![aesthetic]; + } + + // Collect primary + all variants that map to this primary + let mut family = vec![primary]; + for (variant, prim) in AESTHETIC_FAMILIES { + if *prim == primary { + family.push(*variant); + } + } + + family +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_primary_positional() { + assert!(is_primary_positional("x")); + assert!(is_primary_positional("y")); + assert!(!is_primary_positional("xmin")); + assert!(!is_primary_positional("color")); + } + + #[test] + fn test_positional_aesthetic() { + // Primary + assert!(is_positional_aesthetic("x")); + assert!(is_positional_aesthetic("y")); + + // Variants + assert!(is_positional_aesthetic("xmin")); + assert!(is_positional_aesthetic("xmax")); + assert!(is_positional_aesthetic("ymin")); + assert!(is_positional_aesthetic("ymax")); + assert!(is_positional_aesthetic("xend")); + assert!(is_positional_aesthetic("yend")); + + // Non-positional + assert!(!is_positional_aesthetic("color")); + assert!(!is_positional_aesthetic("size")); + assert!(!is_positional_aesthetic("fill")); + } + + #[test] + fn test_all_positional_contents() { + let all: Vec<&str> = ALL_POSITIONAL.iter().map(|s| s.as_str()).collect(); + assert!(all.contains(&"x")); + assert!(all.contains(&"y")); + assert!(all.contains(&"xmin")); + assert!(all.contains(&"xmax")); + assert!(all.contains(&"ymin")); + assert!(all.contains(&"ymax")); + assert!(all.contains(&"xend")); + assert!(all.contains(&"yend")); + assert_eq!(all.len(), 8); // 2 primary * 4 suffixes + } + + #[test] + fn test_is_aesthetic_name() { + // Positional + assert!(is_aesthetic_name("x")); + assert!(is_aesthetic_name("y")); + assert!(is_aesthetic_name("xmin")); + assert!(is_aesthetic_name("yend")); + + // Visual + assert!(is_aesthetic_name("color")); + assert!(is_aesthetic_name("colour")); + assert!(is_aesthetic_name("fill")); + assert!(is_aesthetic_name("stroke")); + assert!(is_aesthetic_name("opacity")); + assert!(is_aesthetic_name("size")); + assert!(is_aesthetic_name("shape")); + assert!(is_aesthetic_name("linetype")); + assert!(is_aesthetic_name("linewidth")); + + // Text + assert!(is_aesthetic_name("label")); + assert!(is_aesthetic_name("family")); + assert!(is_aesthetic_name("fontface")); + assert!(is_aesthetic_name("hjust")); + assert!(is_aesthetic_name("vjust")); + + // Not aesthetics + assert!(!is_aesthetic_name("foo")); + assert!(!is_aesthetic_name("data")); + assert!(!is_aesthetic_name("z")); + } + + #[test] + fn test_primary_aesthetic() { + // Primary aesthetics return themselves + assert_eq!(primary_aesthetic("x"), "x"); + assert_eq!(primary_aesthetic("y"), "y"); + assert_eq!(primary_aesthetic("color"), "color"); + assert_eq!(primary_aesthetic("fill"), "fill"); + + // Variants return their primary + assert_eq!(primary_aesthetic("xmin"), "x"); + assert_eq!(primary_aesthetic("xmax"), "x"); + assert_eq!(primary_aesthetic("xend"), "x"); + assert_eq!(primary_aesthetic("ymin"), "y"); + assert_eq!(primary_aesthetic("ymax"), "y"); + assert_eq!(primary_aesthetic("yend"), "y"); + } + + #[test] + fn test_get_aesthetic_family() { + // Primary aesthetics return full family + let x_family = get_aesthetic_family("x"); + assert!(x_family.contains(&"x")); + assert!(x_family.contains(&"xmin")); + assert!(x_family.contains(&"xmax")); + assert!(x_family.contains(&"xend")); + assert_eq!(x_family.len(), 4); + + let y_family = get_aesthetic_family("y"); + assert!(y_family.contains(&"y")); + assert!(y_family.contains(&"ymin")); + assert!(y_family.contains(&"ymax")); + assert!(y_family.contains(&"yend")); + assert_eq!(y_family.len(), 4); + + // Variants return just themselves + assert_eq!(get_aesthetic_family("xmin"), vec!["xmin"]); + assert_eq!(get_aesthetic_family("ymax"), vec!["ymax"]); + + // Non-family aesthetics return just themselves + assert_eq!(get_aesthetic_family("color"), vec!["color"]); + assert_eq!(get_aesthetic_family("fill"), vec!["fill"]); + } +} diff --git a/src/plot/layer/geom/bar.rs b/src/plot/layer/geom/bar.rs index 1c20fcc6..7ab7e2bf 100644 --- a/src/plot/layer/geom/bar.rs +++ b/src/plot/layer/geom/bar.rs @@ -36,7 +36,7 @@ impl GeomTrait for Bar { &[ ("y", DefaultAestheticValue::Column("count")), ("x", DefaultAestheticValue::Column("x")), - ("y2", DefaultAestheticValue::Number(0.0)), + ("yend", DefaultAestheticValue::Number(0.0)), ] } diff --git a/src/plot/layer/geom/boxplot.rs b/src/plot/layer/geom/boxplot.rs index 877a67c4..de96b683 100644 --- a/src/plot/layer/geom/boxplot.rs +++ b/src/plot/layer/geom/boxplot.rs @@ -36,7 +36,7 @@ impl GeomTrait for Boxplot { ], required: &["x", "y"], // Internal aesthetics produced by stat transform - hidden: &["type", "y", "y2"], + hidden: &["type", "y", "yend"], } } @@ -68,7 +68,7 @@ impl GeomTrait for Boxplot { fn default_remappings(&self) -> &'static [(&'static str, DefaultAestheticValue)] { &[ ("y", DefaultAestheticValue::Column("value")), - ("y2", DefaultAestheticValue::Column("value2")), + ("yend", DefaultAestheticValue::Column("value2")), ("type", DefaultAestheticValue::Column("type")), ] } @@ -272,7 +272,7 @@ fn boxplot_sql_append_outliers( let groups_str = groups.join(", "); // Helper to build visual-element rows from summary table - // Each row type maps to one visual element with y and y2 where needed + // Each row type maps to one visual element with y and yend where needed let build_summary_select = |table: &str| { format!( "SELECT {groups}, 'lower_whisker' AS {type_name}, q1 AS {value_name}, lower AS {value2_name} FROM {table} @@ -695,7 +695,7 @@ mod tests { assert_eq!(remappings.len(), 3); assert!(remappings.contains(&("y", DefaultAestheticValue::Column("value")))); - assert!(remappings.contains(&("y2", DefaultAestheticValue::Column("value2")))); + assert!(remappings.contains(&("yend", DefaultAestheticValue::Column("value2")))); assert!(remappings.contains(&("type", DefaultAestheticValue::Column("type")))); } diff --git a/src/plot/layer/geom/histogram.rs b/src/plot/layer/geom/histogram.rs index 5e3f1300..795f35f0 100644 --- a/src/plot/layer/geom/histogram.rs +++ b/src/plot/layer/geom/histogram.rs @@ -23,17 +23,17 @@ impl GeomTrait for Histogram { GeomAesthetics { supported: &["x", "weight", "fill", "stroke", "opacity"], required: &["x"], - // y and x2 are produced by stat_histogram but not valid for manual MAPPING - hidden: &["y", "x2"], + // y and xend are produced by stat_histogram but not valid for manual MAPPING + hidden: &["y", "xend"], } } fn default_remappings(&self) -> &'static [(&'static str, DefaultAestheticValue)] { &[ ("x", DefaultAestheticValue::Column("bin")), - ("x2", DefaultAestheticValue::Column("bin_end")), + ("xend", DefaultAestheticValue::Column("bin_end")), ("y", DefaultAestheticValue::Column("count")), - ("y2", DefaultAestheticValue::Number(0.0)), + ("yend", DefaultAestheticValue::Number(0.0)), ] } diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 0ef8763e..a0e4456c 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -51,10 +51,10 @@ mod violin; mod vline; // Re-export types -pub use types::{ - get_aesthetic_family, DefaultParam, DefaultParamValue, GeomAesthetics, StatResult, - AESTHETIC_FAMILIES, -}; +pub use types::{DefaultParam, DefaultParamValue, GeomAesthetics, StatResult}; + +// Re-export aesthetic family utilities from the central module +pub use crate::plot::aesthetic::{get_aesthetic_family, AESTHETIC_FAMILIES}; // Re-export geom structs for direct access if needed pub use abline::AbLine; diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index 274b9ba4..0c9f093a 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -2,24 +2,9 @@ //! //! These types are used by all geom implementations and are shared across the module. +use crate::plot::aesthetic::primary_aesthetic; use crate::Mappings; -/// Maps variant aesthetics to their primary aesthetic family. -/// -/// For example, `xmin`, `xmax`, `x2`, and `xend` all belong to the "x" family. -/// When computing labels, all family members can contribute to the primary aesthetic's label, -/// with the first aesthetic encountered in a family setting the label. -pub const AESTHETIC_FAMILIES: &[(&str, &str)] = &[ - ("x2", "x"), - ("xmin", "x"), - ("xmax", "x"), - ("xend", "x"), - ("y2", "y"), - ("ymin", "y"), - ("ymax", "y"), - ("yend", "y"), -]; - /// Aesthetic information for a geom type /// /// This struct describes which aesthetics a geom supports, requires, and hides. @@ -39,43 +24,11 @@ impl GeomAesthetics { /// /// Returns the primary family aesthetic if the input is a variant (e.g., "xmin" -> "x"), /// or returns the aesthetic itself if it's already primary (e.g., "x" -> "x", "fill" -> "fill"). + /// + /// This is a convenience method that delegates to [`crate::plot::aesthetic::primary_aesthetic`]. pub fn primary_aesthetic(aesthetic: &str) -> &str { - AESTHETIC_FAMILIES - .iter() - .find(|(variant, _)| *variant == aesthetic) - .map(|(_, primary)| *primary) - .unwrap_or(aesthetic) - } -} - -/// Get all aesthetics in the same family as the given aesthetic. -/// -/// For primary aesthetics like "x", returns all family members: `["x", "xmin", "xmax", "x2", "xend"]`. -/// For variant aesthetics like "xmin", returns just `["xmin"]` since scales should be -/// defined for primary aesthetics. -/// For non-family aesthetics like "color", returns just `["color"]`. -/// -/// This is used by scale resolution to find all columns that contribute to a scale's -/// input range (e.g., both `ymin` and `ymax` columns contribute to the "y" scale). -pub fn get_aesthetic_family(aesthetic: &str) -> Vec<&str> { - // First, determine the primary aesthetic - let primary = GeomAesthetics::primary_aesthetic(aesthetic); - - // If aesthetic is not a primary (it's a variant), just return the aesthetic itself - // since scales should be defined for primary aesthetics - if primary != aesthetic { - return vec![aesthetic]; + primary_aesthetic(aesthetic) } - - // Collect primary + all variants that map to this primary - let mut family = vec![primary]; - for (variant, prim) in AESTHETIC_FAMILIES { - if *prim == primary { - family.push(*variant); - } - } - - family } /// Default value for a layer parameter diff --git a/src/plot/main.rs b/src/plot/main.rs index 9b51c836..d3613048 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -133,7 +133,7 @@ impl Plot { /// For each aesthetic used in any layer, determines the appropriate label: /// - If user specified a label via LABEL clause, use that /// - Otherwise, use the primary aesthetic's column name if mapped - /// - Variant aesthetics (x2, xmin, xmax, y2, ymin, ymax) only set the label if + /// - Variant aesthetics (xmin, xmax, xend, ymin, ymax, yend) only set the label if /// no primary aesthetic exists in the layer /// /// This ensures that: @@ -437,12 +437,10 @@ mod tests { assert_eq!(GeomAesthetics::primary_aesthetic("x"), "x"); assert_eq!(GeomAesthetics::primary_aesthetic("xmin"), "x"); assert_eq!(GeomAesthetics::primary_aesthetic("xmax"), "x"); - assert_eq!(GeomAesthetics::primary_aesthetic("x2"), "x"); assert_eq!(GeomAesthetics::primary_aesthetic("xend"), "x"); assert_eq!(GeomAesthetics::primary_aesthetic("y"), "y"); assert_eq!(GeomAesthetics::primary_aesthetic("ymin"), "y"); assert_eq!(GeomAesthetics::primary_aesthetic("ymax"), "y"); - assert_eq!(GeomAesthetics::primary_aesthetic("y2"), "y"); assert_eq!(GeomAesthetics::primary_aesthetic("yend"), "y"); // Non-family aesthetics return themselves diff --git a/src/plot/mod.rs b/src/plot/mod.rs index 5dc03ad7..85227baa 100644 --- a/src/plot/mod.rs +++ b/src/plot/mod.rs @@ -15,6 +15,7 @@ //! - `facet` - Facet types for small multiples //! - `coord` - Coordinate system types +pub mod aesthetic; pub mod coord; pub mod facet; pub mod layer; @@ -23,6 +24,7 @@ pub mod scale; pub mod types; // Re-export all types for convenience +pub use aesthetic::*; pub use coord::*; pub use facet::*; pub use layer::*; diff --git a/src/plot/scale/mod.rs b/src/plot/scale/mod.rs index 1385f527..7c22f369 100644 --- a/src/plot/scale/mod.rs +++ b/src/plot/scale/mod.rs @@ -44,7 +44,7 @@ pub fn gets_default_scale(aesthetic: &str) -> bool { matches!( aesthetic, // Position aesthetics - "x" | "y" | "xmin" | "xmax" | "ymin" | "ymax" | "xend" | "yend" | "x2" | "y2" + "x" | "y" | "xmin" | "xmax" | "ymin" | "ymax" | "xend" | "yend" // Color aesthetics (color/colour/col already split to fill/stroke) | "fill" | "stroke" // Size aesthetics diff --git a/src/plot/scale/scale_type/mod.rs b/src/plot/scale/scale_type/mod.rs index 86d23018..9a6de218 100644 --- a/src/plot/scale/scale_type/mod.rs +++ b/src/plot/scale/scale_type/mod.rs @@ -26,6 +26,7 @@ use std::collections::HashMap; use std::sync::Arc; use super::transform::{Transform, TransformKind}; +use crate::plot::aesthetic::is_positional_aesthetic; use crate::plot::{ArrayElement, ColumnInfo, ParameterValue}; // Scale type implementations @@ -1318,15 +1319,6 @@ impl<'de> Deserialize<'de> for ScaleType { // Shared helpers for input range resolution // ============================================================================= -/// Check if an aesthetic is a positional aesthetic (x, y, and variants). -/// Positional aesthetics support properties like `expand`. -pub(super) fn is_positional_aesthetic(aesthetic: &str) -> bool { - matches!( - aesthetic, - "x" | "y" | "xmin" | "xmax" | "ymin" | "ymax" | "xend" | "yend" - ) -} - /// Check if input range contains any Null placeholders pub(crate) fn input_range_has_nulls(range: &[ArrayElement]) -> bool { range.iter().any(|e| matches!(e, ArrayElement::Null)) @@ -2348,11 +2340,13 @@ mod tests { #[test] fn test_expand_positional_vs_non_positional() { + use crate::plot::aesthetic::ALL_POSITIONAL; + let mut props = HashMap::new(); props.insert("expand".to_string(), ParameterValue::Number(0.1)); // Positional aesthetics should allow expand - for aes in &["x", "y", "xmin", "ymax"] { + for aes in ALL_POSITIONAL.iter() { assert!( ScaleType::continuous() .resolve_properties(aes, &props) @@ -2375,10 +2369,12 @@ mod tests { #[test] fn test_oob_defaults_by_aesthetic_type() { + use crate::plot::aesthetic::ALL_POSITIONAL; + let props = HashMap::new(); // Positional aesthetics default to 'keep' - for aesthetic in &["x", "y", "xmin", "xmax", "ymin", "ymax", "xend", "yend"] { + for aesthetic in ALL_POSITIONAL.iter() { let resolved = ScaleType::continuous() .resolve_properties(aesthetic, &props) .unwrap(); diff --git a/src/writer/vegalite/coord.rs b/src/writer/vegalite/coord.rs index 59d05d88..30b5a374 100644 --- a/src/writer/vegalite/coord.rs +++ b/src/writer/vegalite/coord.rs @@ -3,6 +3,7 @@ //! This module handles coordinate system transformations (cartesian, flip, polar) //! that modify the Vega-Lite spec structure based on the COORD clause. +use crate::plot::aesthetic::is_aesthetic_name; use crate::plot::{Coord, CoordType, ParameterValue}; use crate::{DataFrame, GgsqlError, Plot, Result}; use serde_json::{json, Value}; @@ -290,32 +291,3 @@ fn apply_aesthetic_input_range( Ok(()) } - -fn is_aesthetic_name(name: &str) -> bool { - matches!( - name, - "x" | "y" - | "xmin" - | "xmax" - | "ymin" - | "ymax" - | "xend" - | "yend" - | "color" - | "colour" - | "fill" - | "stroke" - | "opacity" - | "size" - | "shape" - | "linetype" - | "linewidth" - | "width" - | "height" - | "label" - | "family" - | "fontface" - | "hjust" - | "vjust" - ) -} diff --git a/src/writer/vegalite/data.rs b/src/writer/vegalite/data.rs index 25dfd370..67730698 100644 --- a/src/writer/vegalite/data.rs +++ b/src/writer/vegalite/data.rs @@ -9,7 +9,7 @@ use crate::plot::scale::ScaleTypeKind; #[allow(unused_imports)] use crate::plot::ArrayElement; use crate::plot::ParameterValue; -use crate::{naming, AestheticValue, DataFrame, GgsqlError, Plot, Result}; +use crate::{is_primary_positional, naming, AestheticValue, DataFrame, GgsqlError, Plot, Result}; use polars::prelude::*; use serde_json::{json, Map, Value}; use std::collections::HashMap; @@ -307,7 +307,7 @@ pub(super) fn collect_binned_columns(spec: &Plot) -> HashMap> { for scale in &spec.scales { // Only x and y aesthetics support bin ranges (x2/y2) in Vega-Lite - if scale.aesthetic != "x" && scale.aesthetic != "y" { + if !is_primary_positional(&scale.aesthetic) { continue; } diff --git a/src/writer/vegalite/encoding.rs b/src/writer/vegalite/encoding.rs index c25cd8da..c80ab184 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -3,10 +3,11 @@ //! This module handles building Vega-Lite encoding channels from ggsql aesthetic mappings, //! including type inference, scale properties, and title handling. +use crate::plot::aesthetic::is_positional_aesthetic; use crate::plot::layer::geom::GeomAesthetics; use crate::plot::scale::{linetype_to_stroke_dash, shape_to_svg_path, ScaleTypeKind}; use crate::plot::ParameterValue; -use crate::{AestheticValue, DataFrame, Plot, Result}; +use crate::{is_primary_positional, AestheticValue, DataFrame, Plot, Result}; use polars::prelude::*; use serde_json::{json, Value}; use std::collections::{HashMap, HashSet}; @@ -170,10 +171,7 @@ pub(super) fn count_binned_legend_scales(spec: &Plot) -> usize { .unwrap_or(false); // Check if non-positional (legend aesthetic) - let is_legend_aesthetic = !matches!( - scale.aesthetic.as_str(), - "x" | "y" | "xmin" | "xmax" | "ymin" | "ymax" | "xend" | "yend" - ); + let is_legend_aesthetic = !is_positional_aesthetic(&scale.aesthetic); is_binned && is_legend_aesthetic }) @@ -252,14 +250,6 @@ pub(super) fn determine_field_type_from_scale( // Phase 1: Utility Helpers // ============================================================================= -/// Check if an aesthetic is positional (maps to an axis rather than a legend) -fn is_positional_aesthetic(aesthetic: &str) -> bool { - matches!( - aesthetic, - "x" | "y" | "xmin" | "xmax" | "ymin" | "ymax" | "xend" | "yend" - ) -} - /// Legend display style for binned scales #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum LegendStyle { @@ -842,7 +832,7 @@ fn build_column_encoding( }; // Position scales don't include zero by default - if aesthetic == "x" || aesthetic == "y" { + if is_primary_positional(aesthetic) { scale_obj.insert("zero".to_string(), json!(false)); } @@ -890,6 +880,9 @@ fn build_literal_encoding(aesthetic: &str, lit: &ParameterValue) -> Result String { match aesthetic { + // Position end aesthetics (ggplot2 style -> Vega-Lite style) + "xend" => "x2", + "yend" => "y2", // Line aesthetics "linetype" => "strokeDash", "linewidth" => "strokeWidth", diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index 0183e568..90669363 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -334,7 +334,7 @@ impl BoxplotRenderer { let type_col = type_col.as_str(); let value_col = naming::aesthetic_column("y"); let value_col = value_col.as_str(); - let value2_col = naming::aesthetic_column("y2"); + let value2_col = naming::aesthetic_column("yend"); let value2_col = value2_col.as_str(); // Find grouping columns (all columns except type, value, value2) @@ -401,7 +401,7 @@ impl BoxplotRenderer { let mut layers: Vec = Vec::new(); let value_col = naming::aesthetic_column("y"); - let value2_col = naming::aesthetic_column("y2"); + let value2_col = naming::aesthetic_column("yend"); let x_col = layer .mappings diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index aa404d8a..4ec095cd 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -31,7 +31,7 @@ use crate::plot::layer::geom::GeomAesthetics; use crate::plot::ArrayElement; use crate::plot::ParameterValue; use crate::writer::Writer; -use crate::{naming, AestheticValue, DataFrame, GgsqlError, Plot, Result}; +use crate::{is_primary_positional, naming, AestheticValue, DataFrame, GgsqlError, Plot, Result}; use serde_json::{json, Value}; use std::collections::HashMap; @@ -235,12 +235,13 @@ fn build_layer_encoding( let channel_encoding = build_encoding_channel(aesthetic, value, &mut enc_ctx)?; encoding.insert(channel_name, channel_encoding); - // For binned positional aesthetics (x, y), add x2/y2 channel with bin_end column - // This enables proper bin width rendering in Vega-Lite - if matches!(aesthetic.as_str(), "x" | "y") && is_binned_aesthetic(aesthetic, spec) { + // For binned positional aesthetics (x, y), add xend/yend channel with bin_end column + // This enables proper bin width rendering in Vega-Lite (maps to x2/y2 channels) + if is_primary_positional(aesthetic) && is_binned_aesthetic(aesthetic, spec) { if let AestheticValue::Column { name: col, .. } = value { let end_col = naming::bin_end_column(col); - let end_channel = format!("{}2", aesthetic); // "x2" or "y2" + let end_aesthetic = format!("{}end", aesthetic); // "xend" or "yend" + let end_channel = map_aesthetic_name(&end_aesthetic); // maps to "x2" or "y2" encoding.insert(end_channel, json!({"field": end_col})); } } @@ -533,7 +534,10 @@ mod tests { assert_eq!(map_aesthetic_name("opacity"), "opacity"); assert_eq!(map_aesthetic_name("size"), "size"); assert_eq!(map_aesthetic_name("shape"), "shape"); - // Mapped aesthetics + // Position end aesthetics (ggsql -> Vega-Lite) + assert_eq!(map_aesthetic_name("xend"), "x2"); + assert_eq!(map_aesthetic_name("yend"), "y2"); + // Other mapped aesthetics assert_eq!(map_aesthetic_name("linetype"), "strokeDash"); assert_eq!(map_aesthetic_name("linewidth"), "strokeWidth"); assert_eq!(map_aesthetic_name("label"), "text"); From 8ab2d47680044609790b3593befecf64b46068bf Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Wed, 18 Feb 2026 13:45:42 +0100 Subject: [PATCH 2/2] update with comments from review --- src/execute/mod.rs | 5 +- src/execute/scale.rs | 9 ++-- src/lib.rs | 2 +- src/plot/aesthetic.rs | 92 ++++++++++++++------------------- src/plot/layer/geom/types.rs | 13 ----- src/plot/main.rs | 26 +++++----- src/writer/vegalite/data.rs | 8 +-- src/writer/vegalite/encoding.rs | 9 ++-- src/writer/vegalite/mod.rs | 8 +-- 9 files changed, 75 insertions(+), 97 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 37fedd78..68bde8fa 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -23,8 +23,7 @@ pub use schema::TypeInfo; use crate::naming; use crate::parser; -use crate::plot::aesthetic::ALL_POSITIONAL; -use crate::plot::layer::geom::GeomAesthetics; +use crate::plot::aesthetic::{primary_aesthetic, ALL_POSITIONAL}; use crate::plot::{AestheticValue, Layer, Scale, ScaleTypeKind, Schema}; use crate::{DataFrame, GgsqlError, Plot, Result}; use std::collections::{HashMap, HashSet}; @@ -326,7 +325,7 @@ fn add_discrete_columns_to_partition_by( // // Discrete and Binned scales produce categorical groupings. // Continuous scales don't group. Identity defers to column type. - let primary_aesthetic = GeomAesthetics::primary_aesthetic(aesthetic); + let primary_aesthetic = primary_aesthetic(aesthetic); let is_discrete = if let Some(scale) = scale_map.get(primary_aesthetic) { if let Some(ref scale_type) = scale.scale_type { match scale_type.scale_type_kind() { diff --git a/src/execute/scale.rs b/src/execute/scale.rs index 1936053c..630f622b 100644 --- a/src/execute/scale.rs +++ b/src/execute/scale.rs @@ -5,7 +5,8 @@ //! and out-of-bounds (OOB) handling. use crate::naming; -use crate::plot::layer::geom::{get_aesthetic_family, GeomAesthetics}; +use crate::plot::aesthetic::primary_aesthetic; +use crate::plot::layer::geom::get_aesthetic_family; use crate::plot::scale::{ default_oob, gets_default_scale, infer_scale_target_type, infer_transform_from_input_range, transform::Transform, OOB_CENSOR, OOB_KEEP, OOB_SQUISH, @@ -32,11 +33,11 @@ pub fn create_missing_scales(spec: &mut Plot) { // (global mappings have already been merged into layers at this point) for layer in &spec.layers { for aesthetic in layer.mappings.aesthetics.keys() { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); used_aesthetics.insert(primary.to_string()); } for aesthetic in layer.remappings.aesthetics.keys() { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); used_aesthetics.insert(primary.to_string()); } } @@ -73,7 +74,7 @@ pub fn create_missing_scales_post_stat(spec: &mut Plot) { // Collect all aesthetics currently in layer mappings for layer in &spec.layers { for aesthetic in layer.mappings.aesthetics.keys() { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); current_aesthetics.insert(primary.to_string()); } } diff --git a/src/lib.rs b/src/lib.rs index 3a25705c..407c3846 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,7 +60,7 @@ pub use plot::{ // Re-export aesthetic classification utilities pub use plot::aesthetic::{ get_aesthetic_family, is_aesthetic_name, is_positional_aesthetic, is_primary_positional, - primary_aesthetic, AESTHETIC_FAMILIES, ALL_POSITIONAL, PRIMARY_POSITIONAL, + primary_aesthetic, AESTHETIC_FAMILIES, ALL_POSITIONAL, NON_POSITIONAL, PRIMARY_POSITIONAL, }; // Future modules - not yet implemented diff --git a/src/plot/aesthetic.rs b/src/plot/aesthetic.rs index adfd446b..e4f3471a 100644 --- a/src/plot/aesthetic.rs +++ b/src/plot/aesthetic.rs @@ -16,13 +16,11 @@ //! For example, `xmin`, `xmax`, and `xend` all belong to the "x" family. //! This is used for scale resolution and label computation. -use std::sync::LazyLock; - /// Primary positional aesthetics (x and y only) pub const PRIMARY_POSITIONAL: &[&str] = &["x", "y"]; -/// Suffixes that create positional variants (e.g., x + "min" = "xmin") -pub const POSITIONAL_SUFFIXES: &[&str] = &["", "min", "max", "end"]; +/// All positional aesthetics (primary + variants) +pub const ALL_POSITIONAL: &[&str] = &["x", "xmin", "xmax", "xend", "y", "ymin", "ymax", "yend"]; /// Maps variant aesthetics to their primary aesthetic family. /// @@ -38,17 +36,31 @@ pub const AESTHETIC_FAMILIES: &[(&str, &str)] = &[ ("yend", "y"), ]; -/// All positional aesthetics (primary + variants), computed from PRIMARY_POSITIONAL x POSITIONAL_SUFFIXES -pub static ALL_POSITIONAL: LazyLock> = LazyLock::new(|| { - PRIMARY_POSITIONAL - .iter() - .flat_map(|&base| { - POSITIONAL_SUFFIXES - .iter() - .map(move |&suffix| format!("{}{}", base, suffix)) - }) - .collect() -}); +/// Non-positional aesthetics (visual properties shown in legends or applied to marks) +/// +/// These include: +/// - Color aesthetics: color, colour, fill, stroke, opacity +/// - Size/shape aesthetics: size, shape, linetype, linewidth +/// - Dimension aesthetics: width, height +/// - Text aesthetics: label, family, fontface, hjust, vjust +pub const NON_POSITIONAL: &[&str] = &[ + "color", + "colour", + "fill", + "stroke", + "opacity", + "size", + "shape", + "linetype", + "linewidth", + "width", + "height", + "label", + "family", + "fontface", + "hjust", + "vjust", +]; /// Check if aesthetic is primary positional (x or y only) #[inline] @@ -61,8 +73,8 @@ pub fn is_primary_positional(aesthetic: &str) -> bool { /// Positional aesthetics include x, y, and their variants (xmin, xmax, ymin, ymax, xend, yend). /// These aesthetics map to axis positions rather than legend entries. #[inline] -pub fn is_positional_aesthetic(aesthetic: &str) -> bool { - ALL_POSITIONAL.iter().any(|s| s == aesthetic) +pub fn is_positional_aesthetic(name: &str) -> bool { + ALL_POSITIONAL.contains(&name) } /// Check if name is a recognized aesthetic @@ -70,32 +82,7 @@ pub fn is_positional_aesthetic(aesthetic: &str) -> bool { /// This includes all positional aesthetics plus visual aesthetics like color, size, shape, etc. #[inline] pub fn is_aesthetic_name(name: &str) -> bool { - matches!( - name, - "x" | "y" - | "xmin" - | "xmax" - | "ymin" - | "ymax" - | "xend" - | "yend" - | "color" - | "colour" - | "fill" - | "stroke" - | "opacity" - | "size" - | "shape" - | "linetype" - | "linewidth" - | "width" - | "height" - | "label" - | "family" - | "fontface" - | "hjust" - | "vjust" - ) + is_positional_aesthetic(name) || NON_POSITIONAL.contains(&name) } /// Get the primary aesthetic for a given aesthetic name. @@ -175,16 +162,15 @@ mod tests { #[test] fn test_all_positional_contents() { - let all: Vec<&str> = ALL_POSITIONAL.iter().map(|s| s.as_str()).collect(); - assert!(all.contains(&"x")); - assert!(all.contains(&"y")); - assert!(all.contains(&"xmin")); - assert!(all.contains(&"xmax")); - assert!(all.contains(&"ymin")); - assert!(all.contains(&"ymax")); - assert!(all.contains(&"xend")); - assert!(all.contains(&"yend")); - assert_eq!(all.len(), 8); // 2 primary * 4 suffixes + assert!(ALL_POSITIONAL.contains(&"x")); + assert!(ALL_POSITIONAL.contains(&"y")); + assert!(ALL_POSITIONAL.contains(&"xmin")); + assert!(ALL_POSITIONAL.contains(&"xmax")); + assert!(ALL_POSITIONAL.contains(&"ymin")); + assert!(ALL_POSITIONAL.contains(&"ymax")); + assert!(ALL_POSITIONAL.contains(&"xend")); + assert!(ALL_POSITIONAL.contains(&"yend")); + assert_eq!(ALL_POSITIONAL.len(), 8); } #[test] diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index 0c9f093a..2d4ecc97 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -2,7 +2,6 @@ //! //! These types are used by all geom implementations and are shared across the module. -use crate::plot::aesthetic::primary_aesthetic; use crate::Mappings; /// Aesthetic information for a geom type @@ -19,18 +18,6 @@ pub struct GeomAesthetics { pub hidden: &'static [&'static str], } -impl GeomAesthetics { - /// Get the primary aesthetic for a given aesthetic name. - /// - /// Returns the primary family aesthetic if the input is a variant (e.g., "xmin" -> "x"), - /// or returns the aesthetic itself if it's already primary (e.g., "x" -> "x", "fill" -> "fill"). - /// - /// This is a convenience method that delegates to [`crate::plot::aesthetic::primary_aesthetic`]. - pub fn primary_aesthetic(aesthetic: &str) -> &str { - primary_aesthetic(aesthetic) - } -} - /// Default value for a layer parameter #[derive(Debug, Clone)] pub enum DefaultParamValue { diff --git a/src/plot/main.rs b/src/plot/main.rs index d3613048..c51ff708 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -33,6 +33,8 @@ pub use super::layer::geom::{ DefaultParam, DefaultParamValue, Geom, GeomAesthetics, GeomTrait, GeomType, StatResult, }; +use super::aesthetic::primary_aesthetic; + // Re-export Layer from the layer module pub use super::layer::Layer; @@ -154,7 +156,7 @@ impl Plot { for primaries_only in [true, false] { for layer in &self.layers { for (aesthetic, value) in &layer.mappings.aesthetics { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); let is_primary = aesthetic == primary; // First pass: only primaries; second pass: only variants @@ -434,19 +436,19 @@ mod tests { #[test] fn test_aesthetic_family_primary_lookup() { // Test that variant aesthetics map to their primary - assert_eq!(GeomAesthetics::primary_aesthetic("x"), "x"); - assert_eq!(GeomAesthetics::primary_aesthetic("xmin"), "x"); - assert_eq!(GeomAesthetics::primary_aesthetic("xmax"), "x"); - assert_eq!(GeomAesthetics::primary_aesthetic("xend"), "x"); - assert_eq!(GeomAesthetics::primary_aesthetic("y"), "y"); - assert_eq!(GeomAesthetics::primary_aesthetic("ymin"), "y"); - assert_eq!(GeomAesthetics::primary_aesthetic("ymax"), "y"); - assert_eq!(GeomAesthetics::primary_aesthetic("yend"), "y"); + assert_eq!(primary_aesthetic("x"), "x"); + assert_eq!(primary_aesthetic("xmin"), "x"); + assert_eq!(primary_aesthetic("xmax"), "x"); + assert_eq!(primary_aesthetic("xend"), "x"); + assert_eq!(primary_aesthetic("y"), "y"); + assert_eq!(primary_aesthetic("ymin"), "y"); + assert_eq!(primary_aesthetic("ymax"), "y"); + assert_eq!(primary_aesthetic("yend"), "y"); // Non-family aesthetics return themselves - assert_eq!(GeomAesthetics::primary_aesthetic("color"), "color"); - assert_eq!(GeomAesthetics::primary_aesthetic("size"), "size"); - assert_eq!(GeomAesthetics::primary_aesthetic("fill"), "fill"); + assert_eq!(primary_aesthetic("color"), "color"); + assert_eq!(primary_aesthetic("size"), "size"); + assert_eq!(primary_aesthetic("fill"), "fill"); } #[test] diff --git a/src/writer/vegalite/data.rs b/src/writer/vegalite/data.rs index 67730698..12e3e3d4 100644 --- a/src/writer/vegalite/data.rs +++ b/src/writer/vegalite/data.rs @@ -3,13 +3,15 @@ //! This module handles converting Polars DataFrames to Vega-Lite JSON data values, //! including temporal type handling and binned data transformations. -use crate::plot::layer::geom::GeomAesthetics; use crate::plot::scale::ScaleTypeKind; // ArrayElement is used for temporal parsing #[allow(unused_imports)] use crate::plot::ArrayElement; use crate::plot::ParameterValue; -use crate::{is_primary_positional, naming, AestheticValue, DataFrame, GgsqlError, Plot, Result}; +use crate::{ + is_primary_positional, naming, primary_aesthetic, AestheticValue, DataFrame, GgsqlError, Plot, + Result, +}; use polars::prelude::*; use serde_json::{json, Map, Value}; use std::collections::HashMap; @@ -349,7 +351,7 @@ pub(super) fn collect_binned_columns(spec: &Plot) -> HashMap> { /// Check if an aesthetic has a binned scale in the spec. pub(super) fn is_binned_aesthetic(aesthetic: &str, spec: &Plot) -> bool { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); spec.find_scale(primary) .and_then(|s| s.scale_type.as_ref()) .map(|st| st.scale_type_kind() == ScaleTypeKind::Binned) diff --git a/src/writer/vegalite/encoding.rs b/src/writer/vegalite/encoding.rs index c80ab184..f489d2d7 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -4,10 +4,9 @@ //! including type inference, scale properties, and title handling. use crate::plot::aesthetic::is_positional_aesthetic; -use crate::plot::layer::geom::GeomAesthetics; use crate::plot::scale::{linetype_to_stroke_dash, shape_to_svg_path, ScaleTypeKind}; use crate::plot::ParameterValue; -use crate::{is_primary_positional, AestheticValue, DataFrame, Plot, Result}; +use crate::{is_primary_positional, primary_aesthetic, AestheticValue, DataFrame, Plot, Result}; use polars::prelude::*; use serde_json::{json, Value}; use std::collections::{HashMap, HashSet}; @@ -330,7 +329,7 @@ fn determine_field_type_for_aesthetic( spec: &Plot, identity_scale: &mut bool, ) -> String { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); let inferred = infer_field_type(df, col); if let Some(scale) = spec.find_scale(primary) { @@ -362,7 +361,7 @@ fn apply_title_to_encoding( titled_families: &mut HashSet, primary_aesthetics: &HashSet, ) { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); let is_primary = aesthetic == primary; let primary_exists = primary_aesthetics.contains(primary); @@ -763,7 +762,7 @@ fn build_column_encoding( is_dummy: bool, ctx: &mut EncodingContext, ) -> Result { - let primary = GeomAesthetics::primary_aesthetic(aesthetic); + let primary = primary_aesthetic(aesthetic); let mut identity_scale = false; // Determine field type from scale or infer from data diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 4ec095cd..db49389d 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -25,13 +25,15 @@ mod data; mod encoding; mod layer; -use crate::plot::layer::geom::GeomAesthetics; // 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, AestheticValue, DataFrame, GgsqlError, Plot, Result}; +use crate::{ + is_primary_positional, naming, primary_aesthetic, AestheticValue, DataFrame, GgsqlError, Plot, + Result, +}; use serde_json::{json, Value}; use std::collections::HashMap; @@ -217,7 +219,7 @@ fn build_layer_encoding( .mappings .aesthetics .keys() - .filter(|a| GeomAesthetics::primary_aesthetic(a) == a.as_str()) + .filter(|a| primary_aesthetic(a) == a.as_str()) .cloned() .collect();