diff --git a/CLAUDE.md b/CLAUDE.md index e24b68b8..344f3172 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -336,7 +336,7 @@ pub enum Geom { // Statistical geoms Histogram, Density, Smooth, Boxplot, Violin, // Annotation geoms - Text, Segment, Arrow, Rule, Linear, ErrorBar, + Text, Segment, Arrow, Rule, ErrorBar, } pub enum DataSource { @@ -1219,7 +1219,7 @@ All clauses (MAPPING, SETTING, PARTITION BY, FILTER) are optional. - **Basic**: `point`, `line`, `path`, `bar`, `col`, `area`, `rect`, `polygon`, `ribbon` - **Statistical**: `histogram`, `density`, `smooth`, `boxplot`, `violin` -- **Annotation**: `text`, `label`, `segment`, `arrow`, `rule`, `linear`, `errorbar` +- **Annotation**: `text`, `label`, `segment`, `arrow`, `rule`, `errorbar` **MAPPING Clause** (Aesthetic Mappings): diff --git a/doc/ggsql.xml b/doc/ggsql.xml index 39437676..1d777ab9 100644 --- a/doc/ggsql.xml +++ b/doc/ggsql.xml @@ -143,7 +143,6 @@ segment arrow rule - linear errorbar @@ -174,6 +173,7 @@ italic hjust vjust + slope diff --git a/doc/index.qmd b/doc/index.qmd index 19fca57f..439beca1 100644 --- a/doc/index.qmd +++ b/doc/index.qmd @@ -44,8 +44,8 @@ WHERE island = 'Biscoe' -- Followed by visualization declaration VISUALISE bill_len AS x, bill_dep AS y, body_mass AS fill DRAW point -DRAW linear - MAPPING 0.4 AS coef, -1 AS intercept +PLACE rule + SETTING slope => 0.4, y => -1 SCALE BINNED fill LABEL title => 'Relationship between bill dimensions in 3 species of penguins', diff --git a/doc/syntax/index.qmd b/doc/syntax/index.qmd index d49a43a6..4384b934 100644 --- a/doc/syntax/index.qmd +++ b/doc/syntax/index.qmd @@ -21,7 +21,6 @@ There are many different layers to choose from when visualising your data. Some - [`line`](layer/type/line.qmd) is used to produce lineplots with the data sorted along the x axis. - [`path`](layer/type/path.qmd) is like `line` above but does not sort the data but plot it according to its own order. - [`segment`](layer/type/segment.qmd) connects two points with a line segment. -- [`linear`](layer/type/linear.qmd) draws a long line parameterised by a coefficient and intercept. - [`rule`](layer/type/rule.qmd) draws horizontal and vertical reference lines. - [`area`](layer/type/area.qmd) is used to display series as an area chart. - [`ribbon`](layer/type/ribbon.qmd) is used to display series extrema. diff --git a/doc/syntax/layer/type/linear.qmd b/doc/syntax/layer/type/linear.qmd deleted file mode 100644 index 92f60459..00000000 --- a/doc/syntax/layer/type/linear.qmd +++ /dev/null @@ -1,70 +0,0 @@ ---- -title: "Linear line" ---- - -> Layers are declared with the [`DRAW` clause](../../clause/draw.qmd). Read the documentation for this clause for a thorough description of how to use it. - -The linear layer is used to draw diagonal reference lines based on a coefficient and intercept. This is useful for adding regression lines, diagonal guides, or mathematical functions to plots. The lines extend across the full extent of the x-axis, regardless of the data range. -The layer is named for the following formula: - -$$ -y = a + \beta x -$$ - -Where $a$ is the `intercept` and $\beta$ is the `coef`. - -> The linear layer doesn't have a natural extend along either axes and the scale ranges can thus not be deduced from it. If the linear layer is the only layer in the visualisation you must specify the position scale ranges manually. - -## Aesthetics -The following aesthetics are recognised by the abline layer. - -### Required -* `coef`: The coefficient/slope of the line i.e. the amount $y$ increases for every unit of $x$. -* `intercept`: The intercept where the line crosses the y-axis at $x = 0$. - -### Optional -* `colour`/`stroke`: The colour of the line -* `opacity`: The opacity of the line -* `linewidth`: The width of the line -* `linetype`: The type of the line, i.e. the dashing pattern - -## Settings -* `orientation`: The orientation of the layer, see the [Orientation section](#orientation). One of the following: - * `'aligned'` to align the layer's primary axis with the coordinate system's first axis. - * `'transposed'` to align the layer's primary axis with the coordinate system's second axis. - -## Data transformation -The linear layer does not transform its data but passes it through unchanged. - -## Orientation -Linear layers use the predictor ($x$) for their primary axis and the response $y$ for its secondary axis. Since the primary axis cannot be deduced from the mapping it must be specified using the `orientation` setting. E.g. to create a vertical linear plot, you need to set `orientation => 'transposed'` to indicate that the primary layer axis follows the second axis of the coordinate system. - -## Examples - -Add a simple reference line to a scatterplot: - -```{ggsql} -VISUALISE FROM ggsql:penguins - DRAW point MAPPING bill_len AS x, bill_dep AS y - PLACE linear SETTING coef => 0.4, intercept => -1 -``` - -Add multiple reference lines with different colors from a separate dataset. Note we're mapping from data here, so we use `DRAW` instead of `PLACE`. - -```{ggsql} -WITH lines AS ( - SELECT * FROM (VALUES - (0.4, -1, 'Line A'), - (0.2, 8, 'Line B'), - (0.8, -19, 'Line C') - ) AS t(coef, intercept, label) -) -VISUALISE FROM ggsql:penguins - DRAW point MAPPING bill_len AS x, bill_dep AS y - DRAW linear - MAPPING - coef AS coef, - intercept AS intercept, - label AS colour - FROM lines -``` diff --git a/doc/syntax/layer/type/rule.qmd b/doc/syntax/layer/type/rule.qmd index dbfacd72..96a27020 100644 --- a/doc/syntax/layer/type/rule.qmd +++ b/doc/syntax/layer/type/rule.qmd @@ -6,6 +6,8 @@ title: "Rule" The rule layer is used to draw reference lines perpendicular to an axis at specified values. This is useful for adding thresholds, means, medians, event markers, cutoff dates or other guides to the plot. The lines span the full width or height of the panels. +Additionally, the rule layer can draw diagonal reference lines by specifying a `slope` along with a position (`x` or `y`). The position acts as the intercept term in the linear equation. This is useful for adding regression lines, diagonal guides, or mathematical functions to plots. + > The rule layer doesn't have a natural extent along the secondary axis. The secondary scale range can thus not be deduced from rule layers. If the rule layer is the only layer in the visualisation, you must specify the position scale range manually. ## Aesthetics @@ -15,19 +17,29 @@ The following aesthetics are recognised by the rule layer. * Primary axis (e.g. `x` or `y`): Position along the primary axis. ### Optional +* `slope` The $\beta$ in the equations described below. Defaults to 0. * `colour`/`stroke`: The colour of the line * `opacity`: The opacity of the line * `linewidth`: The width of the line * `linetype`: The type of the line, i.e. the dashing pattern ## Settings -The rule layer has no additional settings. +* `orientation`: The orientation of the layer, see the [Orientation section](#orientation). One of the following: + * `'aligned'` to align the layer's primary axis with the coordinate system's first axis. + * `'transposed'` to align the layer's primary axis with the coordinate system's second axis. ## Data transformation -The rule layer does not transform its data but passes it through unchanged. + +For diagonal lines, the position aesthetic determines the intercept: + +* `y = a, slope = β` creates the line: $y = a + \beta x$ (line passes through $(0, a)$) +* `x = a, slope = β` creates the line: $x = a + \beta y$ (line passes through $(a, 0)$) + +Note that `slope` represents $\Delta x/\Delta y$ (change in x per unit change in y) when using `x` mapping, not the traditional $\Delta y/\Delta x$. +The default slope is 0, creating horizontal lines when `y` is given and vertical lines when `x` is given. ## Orientation -Rules are drawn perpendicular to their primary axis. The orientation is deduced directly from the mapping. To create a horizontal rule you map the values to `y` instead of `x` (assuming a default Cartesian coordinate system). +The orientation is deduced directly from the mapping. See the ['Data transformation'](#data-transformation) section for details. ## Examples @@ -69,3 +81,31 @@ VISUALISE DRAW line MAPPING date AS x, temperature AS y DRAW rule MAPPING value AS y, label AS colour FROM thresholds ``` + +Add a diagnoal reference line to a scatterplot by using `slope` + +```{ggsql} +VISUALISE FROM ggsql:penguins + DRAW point MAPPING bill_len AS x, bill_dep AS y + PLACE rule SETTING slope => 0.4, y => -1 +``` + +Add multiple reference lines with different colors from a separate dataset. Note we're mapping from data here, so we use `DRAW` instead of `PLACE`. + +```{ggsql} +WITH lines AS ( + SELECT * FROM (VALUES + (0.4, -1, 'Line A'), + (0.2, 8, 'Line B'), + (0.8, -19, 'Line C') + ) AS t(slope, intercept, label) +) +VISUALISE FROM ggsql:penguins + DRAW point MAPPING bill_len AS x, bill_dep AS y + DRAW rule + MAPPING + slope AS slope, + intercept AS y, + label AS colour + FROM lines +``` \ No newline at end of file diff --git a/ggsql-vscode/syntaxes/ggsql.tmLanguage.json b/ggsql-vscode/syntaxes/ggsql.tmLanguage.json index 788cd42c..ba5962cd 100644 --- a/ggsql-vscode/syntaxes/ggsql.tmLanguage.json +++ b/ggsql-vscode/syntaxes/ggsql.tmLanguage.json @@ -250,7 +250,7 @@ "patterns": [ { "name": "support.type.aesthetic.ggsql", - "match": "\\b(x|y|xmin|xmax|ymin|ymax|xend|yend|weight|color|colour|fill|stroke|opacity|size|shape|linetype|linewidth|width|height|label|typeface|fontweight|italic|hjust|vjust|panel|row|column)\\b" + "match": "\\b(x|y|xmin|xmax|ymin|ymax|xend|yend|weight|color|colour|fill|stroke|opacity|size|shape|linetype|linewidth|width|height|label|typeface|fontweight|italic|hjust|vjust|slope|panel|row|column)\\b" } ] }, @@ -295,7 +295,7 @@ "patterns": [ { "name": "support.type.geom.ggsql", - "match": "\\b(point|line|path|bar|col|area|tile|polygon|ribbon|histogram|density|smooth|boxplot|violin|text|label|segment|arrow|rule|linear|errorbar)\\b" + "match": "\\b(point|line|path|bar|col|area|tile|polygon|ribbon|histogram|density|smooth|boxplot|violin|text|label|segment|arrow|rule|errorbar)\\b" }, { "include": "#common-clause-patterns" } ] @@ -309,7 +309,7 @@ "patterns": [ { "name": "support.type.geom.ggsql", - "match": "\\b(point|line|path|bar|col|area|rect|polygon|ribbon|histogram|density|smooth|boxplot|violin|text|label|segment|arrow|rule|linear|errorbar)\\b" + "match": "\\b(point|line|path|bar|col|area|rect|polygon|ribbon|histogram|density|smooth|boxplot|violin|text|label|segment|arrow|rule|errorbar)\\b" }, { "include": "#common-clause-patterns" } ] diff --git a/src/execute/layer.rs b/src/execute/layer.rs index 4993cf40..88005fec 100644 --- a/src/execute/layer.rs +++ b/src/execute/layer.rs @@ -717,7 +717,15 @@ fn process_annotation_layer(layer: &mut Layer) -> Result { } } - // Step 2: Determine max array length from all annotation parameters + // Step 2: Handle empty annotation_params by adding a dummy column + // This occurs when geoms have no required aesthetics and user provides only + // non-positional scalar parameters (e.g., PLACE rule SETTING stroke => 'red') + if annotation_params.is_empty() { + // Add a dummy column so we can generate a valid VALUES clause + annotation_params.push(("__ggsql_dummy__".to_string(), ParameterValue::Number(1.0))); + } + + // Step 3: Determine max array length from all annotation parameters let mut max_length = 1; for (aesthetic, value) in &annotation_params { @@ -743,7 +751,7 @@ fn process_annotation_layer(layer: &mut Layer) -> Result { } } - // Step 3: Build VALUES clause and create final mappings simultaneously + // Step 4: Build VALUES clause and create final mappings simultaneously let mut columns: Vec> = Vec::new(); let mut column_names = Vec::new(); @@ -766,6 +774,11 @@ fn process_annotation_layer(layer: &mut Layer) -> Result { // the same column→aesthetic renaming pipeline as regular layers column_names.push(aesthetic.clone()); + // Skip creating mappings for dummy columns (they're just for valid SQL) + if aesthetic == "__ggsql_dummy__" { + continue; + } + // Create final mapping directly (no intermediate Literal step) let is_positional = crate::plot::aesthetic::is_positional_aesthetic(aesthetic); let mapping_value = if is_positional { @@ -787,14 +800,14 @@ fn process_annotation_layer(layer: &mut Layer) -> Result { layer.parameters.remove(aesthetic); } - // Step 4: Build VALUES rows + // Step 5: Build VALUES rows let mut rows = Vec::new(); for i in 0..max_length { let values: Vec = columns.iter().map(|col| col[i].to_sql()).collect(); rows.push(format!("({})", values.join(", "))); } - // Step 5: Build complete SQL query + // Step 6: Build complete SQL query let values_clause = rows.join(", "); let column_list = column_names .iter() @@ -1106,4 +1119,60 @@ mod tests { ); assert_eq!(series.len(), 2); } + + #[test] + fn test_annotation_no_required_aesthetics() { + // Rule geom has no required aesthetics, only optional ones + let mut layer = Layer::new(Geom::rule()); + layer.source = Some(DataSource::Annotation); + // Only non-positional, non-required scalar parameters + layer.parameters.insert( + "stroke".to_string(), + ParameterValue::String("red".to_string()), + ); + layer + .parameters + .insert("linewidth".to_string(), ParameterValue::Number(2.0)); + + let result = process_annotation_layer(&mut layer); + + // Should generate valid SQL with a dummy column + match result { + Ok(sql) => { + // Check that SQL is valid (has VALUES with at least one column) + assert!( + !sql.contains("(VALUES ) AS t()"), + "Should not generate empty VALUES clause" + ); + assert!( + sql.contains("VALUES") && sql.contains("AS t("), + "Should have VALUES with at least one column" + ); + // Should contain the dummy column + assert!( + sql.contains("__ggsql_dummy__"), + "Should have dummy column when no data columns exist" + ); + } + Err(e) => { + panic!("Unexpected error: {}", e); + } + } + + // Verify that stroke and linewidth remain in parameters (not moved to mappings) + assert!( + layer.parameters.contains_key("stroke"), + "Non-positional, non-required parameters should stay in parameters" + ); + assert!( + layer.parameters.contains_key("linewidth"), + "Non-positional, non-required parameters should stay in parameters" + ); + + // Verify dummy column is not in mappings + assert!( + !layer.mappings.contains_key("__ggsql_dummy__"), + "Dummy column should not be added to mappings" + ); + } } diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 604dabef..ae5d5153 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -839,6 +839,8 @@ fn collect_layer_required_columns(layer: &Layer, spec: &Plot) -> HashSet /// Prune columns from a DataFrame to only include required columns. /// /// Columns that don't exist in the DataFrame are silently ignored. +/// If no required columns exist in the DataFrame (e.g., annotation layers with only +/// literal aesthetics), returns a 0-column DataFrame with the same row count. fn prune_dataframe(df: &DataFrame, required: &HashSet) -> Result { let columns_to_keep: Vec = df .get_column_names() @@ -848,10 +850,28 @@ fn prune_dataframe(df: &DataFrame, required: &HashSet) -> Result 0.4) + // The row count determines how many marks to draw; aesthetics come from Literal values in mappings + let row_count = df.height(); + + if row_count > 0 { + // Create a 0-column DataFrame with the correct row count + // We do this by creating a dummy column and then dropping it + use polars::prelude::df; + let with_rows = df! { + "__dummy__" => vec![0i32; row_count] + } + .map_err(|e| GgsqlError::InternalError(format!("Failed to create DataFrame: {}", e)))?; + + let result = with_rows.drop("__dummy__").map_err(|e| { + GgsqlError::InternalError(format!("Failed to drop dummy column: {}", e)) + })?; + return Ok(result); + } else { + // 0 rows - just return empty DataFrame + return Ok(DataFrame::default()); + } } df.select(&columns_to_keep) @@ -997,6 +1017,16 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result Result { "segment" => Ok(Geom::segment()), "arrow" => Ok(Geom::arrow()), "rule" => Ok(Geom::rule()), - "linear" => Ok(Geom::linear()), "errorbar" => Ok(Geom::errorbar()), _ => Err(GgsqlError::ParseError(format!( "Unknown geom type: {}", diff --git a/src/plot/layer/geom/linear.rs b/src/plot/layer/geom/linear.rs deleted file mode 100644 index 5867dff8..00000000 --- a/src/plot/layer/geom/linear.rs +++ /dev/null @@ -1,45 +0,0 @@ -//! Linear geom implementation - -use super::{ - DefaultAesthetics, DefaultParamValue, GeomTrait, GeomType, ParamConstraint, ParamDefinition, -}; -use crate::plot::layer::orientation::{ALIGNED, ORIENTATION_VALUES}; -use crate::plot::types::DefaultAestheticValue; - -/// Linear geom - lines with coefficient and intercept -#[derive(Debug, Clone, Copy)] -pub struct Linear; - -impl GeomTrait for Linear { - fn geom_type(&self) -> GeomType { - GeomType::Linear - } - - fn aesthetics(&self) -> DefaultAesthetics { - DefaultAesthetics { - defaults: &[ - ("coef", DefaultAestheticValue::Required), - ("intercept", DefaultAestheticValue::Required), - ("stroke", DefaultAestheticValue::String("black")), - ("linewidth", DefaultAestheticValue::Number(1.0)), - ("opacity", DefaultAestheticValue::Number(1.0)), - ("linetype", DefaultAestheticValue::String("solid")), - ], - } - } - - fn default_params(&self) -> &'static [ParamDefinition] { - const PARAMS: &[ParamDefinition] = &[ParamDefinition { - name: "orientation", - default: DefaultParamValue::String(ALIGNED), - constraint: ParamConstraint::string_option(ORIENTATION_VALUES), - }]; - PARAMS - } -} - -impl std::fmt::Display for Linear { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "linear") - } -} diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index 842476a1..fb7ab189 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -36,7 +36,6 @@ mod density; mod errorbar; mod histogram; mod line; -mod linear; mod path; mod point; mod polygon; @@ -62,7 +61,6 @@ pub use density::Density; pub use errorbar::ErrorBar; pub use histogram::Histogram; pub use line::Line; -pub use linear::Linear; pub use path::Path; pub use point::Point; pub use polygon::Polygon; @@ -98,7 +96,6 @@ pub enum GeomType { Segment, Arrow, Rule, - Linear, ErrorBar, } @@ -122,7 +119,6 @@ impl std::fmt::Display for GeomType { GeomType::Segment => "segment", GeomType::Arrow => "arrow", GeomType::Rule => "rule", - GeomType::Linear => "linear", GeomType::ErrorBar => "errorbar", }; write!(f, "{}", s) @@ -219,6 +215,19 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { Ok(df) } + /// Adjust layer mappings and parameters based on geom-specific logic. + /// + /// This method is called during layer execution to allow geoms to customize + /// how aesthetics and parameters should be treated. The default implementation + /// does nothing. + fn setup_layer( + &self, + _mappings: &mut Mappings, + _parameters: &mut HashMap, + ) -> Result<()> { + Ok(()) + } + /// Returns valid parameter names for SETTING clause. /// /// Combines supported aesthetics with non-aesthetic parameters from default_params. @@ -324,11 +333,6 @@ impl Geom { Self(Arc::new(Rule)) } - /// Create an Linear geom - pub fn linear() -> Self { - Self(Arc::new(Linear)) - } - /// Create an ErrorBar geom pub fn errorbar() -> Self { Self(Arc::new(ErrorBar)) @@ -354,7 +358,6 @@ impl Geom { GeomType::Segment => Self::segment(), GeomType::Arrow => Self::arrow(), GeomType::Rule => Self::rule(), - GeomType::Linear => Self::linear(), GeomType::ErrorBar => Self::errorbar(), } } @@ -426,6 +429,15 @@ impl Geom { self.0.post_process(df, parameters) } + /// Adjust layer mappings and parameters based on geom-specific logic + pub fn setup_layer( + &self, + mappings: &mut Mappings, + parameters: &mut HashMap, + ) -> Result<()> { + self.0.setup_layer(mappings, parameters) + } + /// Get valid settings pub fn valid_settings(&self) -> Vec<&'static str> { self.0.valid_settings() @@ -553,7 +565,6 @@ mod tests { GeomType::Segment, GeomType::Arrow, GeomType::Rule, - GeomType::Linear, GeomType::ErrorBar, ]; @@ -577,7 +588,6 @@ mod tests { | GeomType::Segment | GeomType::Arrow | GeomType::Rule - | GeomType::Linear | GeomType::ErrorBar => {} }; diff --git a/src/plot/layer/geom/rule.rs b/src/plot/layer/geom/rule.rs index 0e1ce286..3c090ea8 100644 --- a/src/plot/layer/geom/rule.rs +++ b/src/plot/layer/geom/rule.rs @@ -16,6 +16,7 @@ impl GeomTrait for Rule { DefaultAesthetics { defaults: &[ ("pos1", DefaultAestheticValue::Null), + ("slope", DefaultAestheticValue::Number(0.0)), ("stroke", DefaultAestheticValue::String("black")), ("linewidth", DefaultAestheticValue::Number(1.0)), ("opacity", DefaultAestheticValue::Number(1.0)), @@ -23,6 +24,44 @@ impl GeomTrait for Rule { ], } } + + fn setup_layer( + &self, + mappings: &mut crate::plot::layer::Mappings, + parameters: &mut std::collections::HashMap, + ) -> crate::Result<()> { + use crate::plot::layer::AestheticValue; + use crate::plot::ParameterValue; + + // For diagonal rules (slope present), convert position aesthetics to AnnotationColumn + // so they don't participate in scale training. The position value is the intercept, + // not the actual extent of the line. + + // Check if slope is present and non-zero (in either mappings or parameters) + let has_diagonal_slope = mappings.get("slope").is_some_and(|mapping| { + !matches!(mapping, AestheticValue::Literal(ParameterValue::Number(n)) if *n == 0.0) + }) || parameters.get("slope").is_some_and(|param| { + !matches!(param, ParameterValue::Number(n) if *n == 0.0) + }); + + if !has_diagonal_slope { + return Ok(()); + } + + // For diagonal rules, convert pos1/pos2 to AnnotationColumn so they don't participate in scale training + // The position value is the intercept, not the actual extent of the line + for aesthetic in ["pos1", "pos2"] { + if let Some(mapping) = mappings.aesthetics.get_mut(aesthetic) { + // Convert Column to AnnotationColumn + if let AestheticValue::Column { name, .. } = &*mapping { + let name = name.clone(); + *mapping = AestheticValue::AnnotationColumn { name }; + } + } + } + + Ok(()) + } } impl std::fmt::Display for Rule { diff --git a/src/plot/main.rs b/src/plot/main.rs index 74360c62..8e61d92b 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -538,12 +538,6 @@ mod tests { // Segment/arrow require endpoints assert_eq!(Geom::segment().aesthetics().required(), &["pos1", "pos2"]); - // Reference lines - assert_eq!( - Geom::linear().aesthetics().required(), - &["coef", "intercept"] - ); - // ErrorBar has no strict requirements assert_eq!(Geom::errorbar().aesthetics().required(), &[] as &[&str]); } diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index dcdc5092..71c1348d 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -43,7 +43,6 @@ pub fn geom_to_mark(geom: &Geom) -> Value { GeomType::Segment => "rule", GeomType::Smooth => "line", GeomType::Rule => "rule", - GeomType::Linear => "rule", GeomType::ErrorBar => "rule", _ => "point", // Default fallback }; @@ -401,77 +400,65 @@ impl GeomRenderer for RuleRenderer { &self, encoding: &mut Map, _layer: &Layer, - _context: &RenderContext, + context: &RenderContext, ) -> Result<()> { let has_x = encoding.contains_key("x"); let has_y = encoding.contains_key("y"); - if !has_x && !has_y { + + // Remove slope from encoding (it's never a visual encoding, only metadata) + // and check if it's non-zero (diagonal line) + let diagonal = if let Some(slope_enc) = encoding.remove("slope") { + slope_enc.get("field").is_some() // Field reference - assume non-zero + || slope_enc + .get("value") + .and_then(|v| v.as_f64()) + .map(|v| v != 0.0) + .unwrap_or(false) // Literal value - check if non-zero + } else { + false + }; + + if !has_x && !has_y && !diagonal { return Err(GgsqlError::ValidationError( "The `rule` layer requires the `x` or `y` aesthetic. It currently has neither." .to_string(), )); - } else if has_x && has_y { + } else if has_x && has_y && !diagonal { return Err(GgsqlError::ValidationError( "The `rule` layer requires exactly one of the `x` or `y` aesthetic, not both." .to_string(), )); } - Ok(()) - } -} - -// ============================================================================= -// Linear Renderer -// ============================================================================= - -/// Renderer for linear geom - draws lines based on coefficient and intercept -pub struct LinearRenderer; - -impl GeomRenderer for LinearRenderer { - fn prepare_data( - &self, - df: &DataFrame, - _layer: &Layer, - _data_key: &str, - _binned_columns: &HashMap>, - ) -> Result { - // Just convert DataFrame to JSON values - // No need to add xmin/xmax - they'll be encoded as literal values - let values = dataframe_to_values(df)?; - Ok(PreparedData::Single { values }) - } - - fn modify_encoding( - &self, - encoding: &mut Map, - layer: &Layer, - _context: &RenderContext, - ) -> Result<()> { - // Remove coefficient and intercept from encoding - they're only used in transforms - encoding.remove("coef"); - encoding.remove("intercept"); + if !diagonal { + return Ok(()); + } - // Check orientation - let is_horizontal = is_transposed(layer); + // Determine orientation from which aesthetic is mapped: + // - If y is mapped: y is intercept, x varies (primary axis is x) + // - If x is mapped: x is intercept, y varies (primary axis is y, "horizontal") + let is_horizontal = has_x && !has_y; - // For aligned (default): x is primary axis, y is computed (secondary) - // For transposed: y is primary axis, x is computed (secondary) + // For y mapped (not horizontal): x is primary axis (varies), y is computed (secondary) + // For x mapped (horizontal): y is primary axis (varies), x is computed (secondary) let (primary, primary2, secondary, secondary2) = if is_horizontal { ("y", "y2", "x", "x2") } else { ("x", "x2", "y", "y2") }; + let mut primary_enco = json!({"field": "primary_min", "type": "quantitative"}); + + // Get primary axis extent from context to set explicit scale domain + // This prevents an axis drift + let extent_aesthetic = if is_horizontal { "pos2" } else { "pos1" }; + if let Ok((min, max)) = context.get_extent(extent_aesthetic) { + primary_enco["scale"] = json!({"domain": [min, max]}) + }; + // Add encodings for rule mark // primary_min/primary_max are created by transforms (extent of the axis) // secondary_min/secondary_max are computed via formula - encoding.insert( - primary.to_string(), - json!({ - "field": "primary_min", - "type": "quantitative" - }), - ); + encoding.insert(primary.to_string(), primary_enco); encoding.insert( primary2.to_string(), json!({ @@ -501,22 +488,56 @@ impl GeomRenderer for LinearRenderer { layer: &Layer, context: &RenderContext, ) -> Result<()> { - // Field names for coef and intercept (with aesthetic column prefix) - let coef_field = naming::aesthetic_column("coef"); - let intercept_field = naming::aesthetic_column("intercept"); + // Determine slope expression: either a literal value or a field reference + let slope_expr = match layer.mappings.get("slope") { + Some(AestheticValue::Literal(ParameterValue::Number(n))) if *n == 0.0 => { + // Slope is 0 - no diagonal + None + } + Some(AestheticValue::Literal(ParameterValue::Number(n))) => { + // Literal non-zero slope - inline the value + Some(n.to_string()) + } + Some(AestheticValue::Column { .. }) | Some(AestheticValue::AnnotationColumn { .. }) => { + // Column-based slope - reference the field + let slope_field = naming::aesthetic_column("slope"); + Some(format!("datum.{}", slope_field)) + } + _ => { + // No slope mapping - no diagonal + None + } + }; - // Check orientation - let is_horizontal = is_transposed(layer); + let Some(slope_expr) = slope_expr else { + return Ok(()); + }; + + // Determine orientation from which positional aesthetic is mapped: + // By this point, x/y have been renamed to pos1/pos2 by resolve_aesthetic + // - If pos2 is mapped (from y): pos2 is intercept, pos1 varies (primary axis is pos1) + // - If pos1 is mapped (from x): pos1 is intercept, pos2 varies (primary axis is pos2, "horizontal") + let has_pos1 = layer.mappings.contains_key("pos1"); + let has_pos2 = layer.mappings.contains_key("pos2"); + let is_horizontal = has_pos1 && !has_pos2; + + // Get the intercept field (pos1 for x, pos2 for y) + let intercept_field = if is_horizontal { + naming::aesthetic_column("pos1") // x is intercept + } else { + naming::aesthetic_column("pos2") // y is intercept + }; // Get extent from appropriate axis: - // - Aligned (default): extent from pos1 (x-axis), compute y from x - // - Transposed: extent from pos2 (y-axis), compute x from y + // - y mapped (not horizontal): x is primary axis (varies), y is computed + // - x mapped (horizontal): y is primary axis (varies), x is computed let extent_aesthetic = if is_horizontal { "pos2" } else { "pos1" }; let (primary_min, primary_max) = context.get_extent(extent_aesthetic)?; // Add transforms: // 1. Create constant primary_min/primary_max fields (extent of the primary axis) - // 2. Compute secondary values at those primary positions: secondary = coef * primary + intercept + // 2. Compute secondary values at those primary positions: secondary = slope * primary + intercept + // (where intercept is pos1 for x-mapped or pos2 for y-mapped) let transforms = json!([ { "calculate": primary_min.to_string(), @@ -527,11 +548,11 @@ impl GeomRenderer for LinearRenderer { "as": "primary_max" }, { - "calculate": format!("datum.{} * datum.primary_min + datum.{}", coef_field, intercept_field), + "calculate": format!("{} * datum.primary_min + datum.{}", slope_expr, intercept_field), "as": "secondary_min" }, { - "calculate": format!("datum.{} * datum.primary_max + datum.{}", coef_field, intercept_field), + "calculate": format!("{} * datum.primary_max + datum.{}", slope_expr, intercept_field), "as": "secondary_max" } ]); @@ -1906,7 +1927,6 @@ pub fn get_renderer(geom: &Geom) -> Box { GeomType::Violin => Box::new(ViolinRenderer), GeomType::Text => Box::new(TextRenderer), GeomType::Segment => Box::new(SegmentRenderer), - GeomType::Linear => Box::new(LinearRenderer), GeomType::ErrorBar => Box::new(ErrorBarRenderer), GeomType::Rule => Box::new(RuleRenderer), // All other geoms (Point, Area, Density, Tile, etc.) use the default renderer @@ -3276,11 +3296,11 @@ mod tests { } #[test] - fn test_linear_renderer_multiple_lines() { + fn test_rule_renderer_multiple_diagonal_lines() { use crate::reader::{DuckDBReader, Reader}; use crate::writer::{VegaLiteWriter, Writer}; - // Test that linear with 3 different coefficients renders 3 separate lines + // Test that rule with 3 different slopes renders 3 separate lines let query = r#" WITH points AS ( SELECT * FROM (VALUES (0, 5), (5, 15), (10, 25)) AS t(x, y) @@ -3290,12 +3310,12 @@ mod tests { (2, 5, 'A'), (1, 10, 'B'), (3, 0, 'C') - ) AS t(coef, intercept, line_id) + ) AS t(slope, y, line_id) ) SELECT * FROM points VISUALISE DRAW point MAPPING x AS x, y AS y - DRAW linear MAPPING coef AS coef, intercept AS intercept, line_id AS color FROM lines + DRAW rule MAPPING slope AS slope, y AS y, line_id AS color FROM lines "#; // Execute query @@ -3311,21 +3331,21 @@ mod tests { let vl_spec: serde_json::Value = serde_json::from_str(&vl_json).expect("Failed to parse Vega-Lite JSON"); - // Verify we have 2 layers (point + linear) + // Verify we have 2 layers (point + rule) let layers = vl_spec["layer"].as_array().expect("No layers found"); - assert_eq!(layers.len(), 2, "Should have 2 layers (point + linear)"); + assert_eq!(layers.len(), 2, "Should have 2 layers (point + rule)"); - // Get the linear layer (second layer) - let linear_layer = &layers[1]; + // Get the rule layer (second layer) + let rule_layer = &layers[1]; // Verify it's a rule mark assert_eq!( - linear_layer["mark"]["type"], "rule", - "Linear should use rule mark" + rule_layer["mark"]["type"], "rule", + "Rule should use rule mark" ); // Verify transforms exist - let transforms = linear_layer["transform"] + let transforms = rule_layer["transform"] .as_array() .expect("No transforms found"); @@ -3355,7 +3375,7 @@ mod tests { "primary_max should have calculate expression" ); - // Verify secondary_min and secondary_max transforms use coef and intercept with primary_min/primary_max + // Verify secondary_min and secondary_max transforms use slope and intercept with primary_min/primary_max let secondary_min_transform = transforms .iter() .find(|t| t["as"] == "secondary_min") @@ -3372,26 +3392,18 @@ mod tests { .as_str() .expect("secondary_max calculate should be string"); - // Should reference coef, intercept, and primary_min/primary_max - assert!( - secondary_min_calc.contains("__ggsql_aes_coef__"), - "secondary_min should reference coef" - ); + // Should reference slope, pos2 (acting as intercept for y-mapped rules), and primary_min/primary_max assert!( - secondary_min_calc.contains("__ggsql_aes_intercept__"), - "secondary_min should reference intercept" + secondary_min_calc.contains("__ggsql_aes_pos2__"), + "secondary_min should reference pos2 (y intercept)" ); assert!( secondary_min_calc.contains("datum.primary_min"), "secondary_min should reference datum.primary_min" ); assert!( - secondary_max_calc.contains("__ggsql_aes_coef__"), - "secondary_max should reference coef" - ); - assert!( - secondary_max_calc.contains("__ggsql_aes_intercept__"), - "secondary_max should reference intercept" + secondary_max_calc.contains("__ggsql_aes_pos2__"), + "secondary_max should reference pos2 (y intercept)" ); assert!( secondary_max_calc.contains("datum.primary_max"), @@ -3399,7 +3411,7 @@ mod tests { ); // Verify encoding has x, x2, y, y2 with consistent field names - let encoding = linear_layer["encoding"] + let encoding = rule_layer["encoding"] .as_object() .expect("No encoding found"); @@ -3432,52 +3444,56 @@ mod tests { "Should have stroke encoding for line_id" ); - // Verify data has 3 linear rows (one per coef) + // Verify data has 3 rule rows (one per slope) let data_values = vl_spec["data"]["values"] .as_array() .expect("No data values found"); - let linear_rows: Vec<_> = data_values + let rule_rows: Vec<_> = data_values .iter() .filter(|row| { row["__ggsql_source__"] == "__ggsql_layer_1__" - && row["__ggsql_aes_coef__"].is_number() + && row["__ggsql_aes_slope__"].is_number() }) .collect(); assert_eq!( - linear_rows.len(), + rule_rows.len(), 3, - "Should have 3 linear rows (3 different coefficients)" + "Should have 3 rule rows (3 different slopes)" ); - // Verify we have coefs 1, 2, 3 - let mut coefs: Vec = linear_rows + // Verify we have slopes 1, 2, 3 + let mut slopes: Vec = rule_rows .iter() - .map(|row| row["__ggsql_aes_coef__"].as_f64().unwrap()) + .map(|row| row["__ggsql_aes_slope__"].as_f64().unwrap()) .collect(); - coefs.sort_by(|a, b| a.partial_cmp(b).unwrap()); + slopes.sort_by(|a, b| a.partial_cmp(b).unwrap()); - assert_eq!(coefs, vec![1.0, 2.0, 3.0], "Should have coefs 1, 2, and 3"); + assert_eq!( + slopes, + vec![1.0, 2.0, 3.0], + "Should have slopes 1, 2, and 3" + ); } #[test] - fn test_linear_renderer_transposed_orientation() { + fn test_sloped_rule_renderer_horizontal_orientation() { use crate::reader::{DuckDBReader, Reader}; use crate::writer::{VegaLiteWriter, Writer}; - // Test that linear with transposed orientation swaps x/y axes + // Test that sloped rule with x mapping (horizontal) infers y varies let query = r#" WITH points AS ( SELECT * FROM (VALUES (0, 5), (5, 15), (10, 25)) AS t(x, y) ), lines AS ( - SELECT * FROM (VALUES (0.4, -1, 'A')) AS t(coef, intercept, line_id) + SELECT * FROM (VALUES (0.4, -1, 'A')) AS t(slope, x, line_id) ) SELECT * FROM points VISUALISE DRAW point MAPPING x AS x, y AS y - DRAW linear MAPPING coef AS coef, intercept AS intercept, line_id AS color FROM lines SETTING orientation => 'transposed' + DRAW rule MAPPING slope AS slope, x AS x, line_id AS color FROM lines "#; // Execute query @@ -3493,16 +3509,16 @@ mod tests { let vl_spec: serde_json::Value = serde_json::from_str(&vl_json).expect("Failed to parse Vega-Lite JSON"); - // Get the linear layer (second layer) + // Get the rule layer (second layer) let layers = vl_spec["layer"].as_array().expect("No layers found"); - let linear_layer = &layers[1]; + let rule_layer = &layers[1]; // Verify transforms exist - let transforms = linear_layer["transform"] + let transforms = rule_layer["transform"] .as_array() .expect("No transforms found"); - // Verify primary_min/max use pos2 extent (y-axis) for transposed orientation + // Verify primary_min/max use pos2 extent (y-axis) for horizontal (x-mapped) orientation let primary_min_transform = transforms .iter() .find(|t| t["as"] == "primary_min") @@ -3512,7 +3528,7 @@ mod tests { .find(|t| t["as"] == "primary_max") .expect("primary_max transform not found"); - // The primary extent should come from the y-axis for transposed + // The primary extent should come from the y-axis for horizontal orientation assert!( primary_min_transform["calculate"].is_string(), "primary_min should have calculate expression" @@ -3522,27 +3538,54 @@ mod tests { "primary_max should have calculate expression" ); + // Verify secondary_min and secondary_max use pos1 (x intercept) for horizontal orientation + let secondary_min_transform = transforms + .iter() + .find(|t| t["as"] == "secondary_min") + .expect("secondary_min transform not found"); + let secondary_max_transform = transforms + .iter() + .find(|t| t["as"] == "secondary_max") + .expect("secondary_max transform not found"); + + let secondary_min_calc = secondary_min_transform["calculate"] + .as_str() + .expect("secondary_min calculate should be string"); + let secondary_max_calc = secondary_max_transform["calculate"] + .as_str() + .expect("secondary_max calculate should be string"); + + // Should reference pos1 (x intercept) for horizontal orientation + assert!( + secondary_min_calc.contains("__ggsql_aes_pos1__"), + "secondary_min should reference pos1 (x intercept)" + ); + assert!( + secondary_max_calc.contains("__ggsql_aes_pos1__"), + "secondary_max should reference pos1 (x intercept)" + ); + // Verify encoding has y as primary axis (mapped to primary_min/max) - let encoding = linear_layer["encoding"] + let encoding = rule_layer["encoding"] .as_object() .expect("No encoding found"); - // For transposed orientation: y is primary (uses primary_min/max), x is secondary + // For horizontal orientation (x-mapped): y is primary (uses primary_min/max), x is secondary assert_eq!( encoding["y"]["field"], "primary_min", - "y should reference primary_min field for transposed" + "y should reference primary_min field for horizontal orientation" ); assert_eq!( encoding["y2"]["field"], "primary_max", - "y2 should reference primary_max field for transposed" + "y2 should reference primary_max field for horizontal orientation" ); assert_eq!( encoding["x"]["field"], "secondary_min", - "x should reference secondary_min field for transposed" + "x should reference secondary_min field for horizontal orientation" ); assert_eq!( encoding["x2"]["field"], "secondary_max", - "x2 should reference secondary_max field for transposed" + "x2 should reference secondary_max field for horizontal orientation" ); } diff --git a/tree-sitter-ggsql/grammar.js b/tree-sitter-ggsql/grammar.js index 1291c4fa..1386ad6d 100644 --- a/tree-sitter-ggsql/grammar.js +++ b/tree-sitter-ggsql/grammar.js @@ -501,7 +501,7 @@ module.exports = grammar({ geom_type: $ => choice( 'point', 'line', 'path', 'bar', 'area', 'rect', 'polygon', 'ribbon', 'histogram', 'density', 'smooth', 'boxplot', 'violin', - 'text', 'label', 'segment', 'arrow', 'rule', 'linear', 'errorbar' + 'text', 'label', 'segment', 'arrow', 'rule', 'errorbar' ), // MAPPING clause for aesthetic mappings: MAPPING col AS x, "blue" AS color [FROM source] @@ -683,7 +683,7 @@ module.exports = grammar({ // Text aesthetics 'label', 'typeface', 'fontweight', 'italic', 'fontsize', 'hjust', 'vjust', 'rotation', // Specialty aesthetics, - 'coef', 'intercept', + 'slope', // Facet aesthetics 'panel', 'row', 'column', // Computed variables diff --git a/tree-sitter-ggsql/queries/highlights.scm b/tree-sitter-ggsql/queries/highlights.scm index 82209424..c7da26fa 100644 --- a/tree-sitter-ggsql/queries/highlights.scm +++ b/tree-sitter-ggsql/queries/highlights.scm @@ -25,7 +25,6 @@ "segment" "arrow" "rule" - "linear" "errorbar" ] @type.builtin @@ -58,6 +57,7 @@ "hjust" "vjust" "rotation" + "slope" "panel" "row" "column"