Consolidate aesthetic information (#135)

Author: thomasp85

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() {
@@ -1063,20 +1059,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::<Vec<_>>()
         );
 
-        // 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(&yend_col).is_ok(),
             "DataFrame should have '{}' column: {:?}",
-            y2_col,
+            yend_col,
             layer_df.get_column_names_str()
         );
     }

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");

src/lib.rs

@@ -55,6 +55,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;

src/naming.rs

@@ -285,21 +285,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)
 }
@@ -452,9 +452,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]

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<CoordType> {
     let text = source.get_text(node);