diff --git a/src/execute/mod.rs b/src/execute/mod.rs index a7fce754..68bde8fa 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -23,7 +23,7 @@ pub use schema::TypeInfo; use crate::naming; use crate::parser; -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}; @@ -285,12 +285,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 +301,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; } @@ -329,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() { @@ -1071,20 +1067,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..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()); } } @@ -1254,15 +1255,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..407c3846 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, NON_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..e4f3471a --- /dev/null +++ b/src/plot/aesthetic.rs @@ -0,0 +1,250 @@ +//! 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. + +/// Primary positional aesthetics (x and y only) +pub const PRIMARY_POSITIONAL: &[&str] = &["x", "y"]; + +/// 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. +/// +/// 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"), +]; + +/// 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] +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(name: &str) -> bool { + ALL_POSITIONAL.contains(&name) +} + +/// 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 { + is_positional_aesthetic(name) || NON_POSITIONAL.contains(&name) +} + +/// 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() { + 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] + 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..2d4ecc97 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -4,22 +4,6 @@ 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. @@ -34,50 +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"). - 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]; - } - - // 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 #[derive(Debug, Clone)] pub enum DefaultParamValue { diff --git a/src/plot/main.rs b/src/plot/main.rs index 9b51c836..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; @@ -133,7 +135,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: @@ -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,21 +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("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"); + 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/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..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::{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; @@ -307,7 +309,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; } @@ -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 c25cd8da..f489d2d7 100644 --- a/src/writer/vegalite/encoding.rs +++ b/src/writer/vegalite/encoding.rs @@ -3,10 +3,10 @@ //! This module handles building Vega-Lite encoding channels from ggsql aesthetic mappings, //! including type inference, scale properties, and title handling. -use crate::plot::layer::geom::GeomAesthetics; +use crate::plot::aesthetic::is_positional_aesthetic; 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, primary_aesthetic, AestheticValue, DataFrame, Plot, Result}; use polars::prelude::*; use serde_json::{json, Value}; use std::collections::{HashMap, HashSet}; @@ -170,10 +170,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 +249,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 { @@ -340,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) { @@ -372,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); @@ -773,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 @@ -842,7 +831,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 +879,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..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::{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(); @@ -235,12 +237,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 +536,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");