Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions src/execute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}

Expand All @@ -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() {
Expand Down Expand Up @@ -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::<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()
);
}
Expand Down
13 changes: 7 additions & 6 deletions src/execute/scale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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());
}
}
Expand Down Expand Up @@ -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());
}
}
Expand Down Expand Up @@ -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");
Expand Down
6 changes: 6 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 8 additions & 8 deletions src/naming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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]
Expand Down
31 changes: 1 addition & 30 deletions src/parser/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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);
Expand Down
Loading