From c64ad0152421e2d88a6b2d0f5dcdd404ca4fa1e4 Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Mon, 9 Mar 2026 12:58:15 +0100 Subject: [PATCH 1/5] Add bidirectionality to layers --- doc/syntax/clause/project.qmd | 2 +- doc/syntax/coord/cartesian.qmd | 4 +- doc/syntax/coord/polar.qmd | 4 +- doc/syntax/layer/area.qmd | 12 +- doc/syntax/layer/bar.qmd | 19 +- doc/syntax/layer/boxplot.qmd | 9 +- doc/syntax/layer/density.qmd | 7 +- doc/syntax/layer/histogram.qmd | 9 +- doc/syntax/layer/line.qmd | 11 +- doc/syntax/layer/path.qmd | 10 +- doc/syntax/layer/point.qmd | 4 +- doc/syntax/layer/polygon.qmd | 6 +- doc/syntax/layer/ribbon.qmd | 10 +- doc/syntax/layer/violin.qmd | 7 +- src/execute/layer.rs | 126 +++++++- src/execute/mod.rs | 149 ++++++++- src/parser/builder.rs | 17 + src/plot/aesthetic.rs | 40 ++- src/plot/layer/geom/area.rs | 26 +- src/plot/layer/geom/boxplot.rs | 3 +- src/plot/layer/geom/density.rs | 5 +- src/plot/layer/geom/histogram.rs | 4 +- src/plot/layer/geom/line.rs | 26 +- src/plot/layer/geom/mod.rs | 3 + src/plot/layer/geom/ribbon.rs | 26 +- src/plot/layer/geom/types.rs | 41 ++- src/plot/layer/mod.rs | 19 +- src/plot/layer/orientation.rs | 524 +++++++++++++++++++++++++++++++ src/plot/main.rs | 6 +- src/writer/vegalite/layer.rs | 214 +++++++++---- src/writer/vegalite/mod.rs | 2 +- 31 files changed, 1199 insertions(+), 146 deletions(-) create mode 100644 src/plot/layer/orientation.rs diff --git a/doc/syntax/clause/project.qmd b/doc/syntax/clause/project.qmd index d7753ce5..141d298a 100644 --- a/doc/syntax/clause/project.qmd +++ b/doc/syntax/clause/project.qmd @@ -12,7 +12,7 @@ PROJECT , ... TO SETTING => , ... ``` -The comma-separated list of `aesthetic` names are optional but allows you to define the names of the positional aesthetics in the plot. If omitted, the default aesthetic names of the coordinate system is used. The order given matters as the first name is used for the primary aesthetic, the second name for the secondary aesthetic and so on. For instance, using `PROJECT y, x TO cartesian` will flip the plot as anything mapped to `y` will now relate to the horizontal axis, and anything mapped to `x` will relate to the vertical axis. Note that it is not allowed to use the name of already established aesthetics as positional aesthetics, e.g. `PROJECT fill, stroke TO polar` is not allowed. +The comma-separated list of `aesthetic` names are optional but allows you to define the names of the positional aesthetics in the plot. If omitted, the default aesthetic names of the coordinate system is used. The order given matters as the first name is used for the 1st axis, the second name for the 2nd axis and so on. For instance, using `PROJECT y, x TO cartesian` will flip the plot as anything mapped to `y` will now relate to the horizontal axis, and anything mapped to `x` will relate to the vertical axis. Note that it is not allowed to use the name of already established aesthetics as positional aesthetics, e.g. `PROJECT fill, stroke TO polar` is not allowed. ### `TO` ```ggsql diff --git a/doc/syntax/coord/cartesian.qmd b/doc/syntax/coord/cartesian.qmd index c807b88d..d4291e2f 100644 --- a/doc/syntax/coord/cartesian.qmd +++ b/doc/syntax/coord/cartesian.qmd @@ -7,8 +7,8 @@ The Cartesian coordinate system is the most well-known and the default for ggsql ## Default aesthetics The Cartesian coordinate system has the following default positional aesthetics which will be used if no others have been provided: -* **Primary**: `x` (horizontal position) -* **Secondary**: `y` (vertical position) +* **First**: `x` (horizontal position) +* **Second**: `y` (vertical position) Users can provide their own aesthetic names if needed, e.g. diff --git a/doc/syntax/coord/polar.qmd b/doc/syntax/coord/polar.qmd index ffa8ae51..dfbf07d7 100644 --- a/doc/syntax/coord/polar.qmd +++ b/doc/syntax/coord/polar.qmd @@ -7,8 +7,8 @@ The polar coordinate system interprets its primary aesthetic as the angular posi ## Default aesthetics The polar coordinate system has the following default positional aesthetics which will be used if no others have been provided: -* **Primary**: `theta` (angular position) -* **Secondary**: `radius` (distance from center) +* **First**: `theta` (angular position) +* **Second**: `radius` (distance from center) Users can provide their own aesthetic names if needed. For example, if using `x` and `y` aesthetics: diff --git a/doc/syntax/layer/area.qmd b/doc/syntax/layer/area.qmd index c247b95d..1000386c 100644 --- a/doc/syntax/layer/area.qmd +++ b/doc/syntax/layer/area.qmd @@ -10,8 +10,8 @@ The area layer is used to display absolute amounts over a sorted x-axis. It can The following aesthetics are recognised by the area layer. ### Required -* `x`: Position along the x-axis. -* `y`: Position along the y-axis. +* Primary axis (e.g. `x`): The value of the independent variable. +* Secondary axis (e.g. `y`): The value of the dependent variable. ### Optional * `stroke`: The colour of the contour lines. @@ -25,9 +25,13 @@ The following aesthetics are recognised by the area layer. * `'off'`: The groups `y`-values are displayed as-is (default). * `'on'`: The `y`-values are stacked per `x` position, accumulating over groups. * `'fill'`: Like `'on'` but displayed as a fraction of the total per `x` position. +* `orientation`: The orientation of the layer. Either `'aligned'` (default), or `'transposed'`. Determines if the layers primary axis is aligned with the coordinate systems first axis or not ## Data transformation -The area layer does not transform its data but passes it through unchanged. +The area layer sorts the data along its primary axis + +## Orientation +Area plots are sorted and connected along their primary axis. Since the primary axis cannot be deduced from the mapping it must be specified using the `orientation` setting. E.g. if you wish to create a vertical area plot you need to set `orientation => 'transposed'` to indicate that the primary layer axis follows the second axis of the coordinate system. ## Examples @@ -69,4 +73,4 @@ When `stacking => 'fill'` we're plotting stacked proportions. These only make se ```{ggsql} VISUALISE Date AS x, Value AS y, Series AS colour FROM long_airquality DRAW area SETTING stacking => 'fill' -``` \ No newline at end of file +``` diff --git a/doc/syntax/layer/bar.qmd b/doc/syntax/layer/bar.qmd index 96948b85..8441da4b 100644 --- a/doc/syntax/layer/bar.qmd +++ b/doc/syntax/layer/bar.qmd @@ -13,8 +13,8 @@ The following aesthetics are recognised by the bar layer. The bar layer has no required aesthetics ### Optional -* `x`: Position on the x-axis. If missing all records will be shown in the same bar -* `y`: The height of the plot. If missing, it will be calculated by the layer +* Primary axis (e.g. `x`): The categories to create bars for. If missing all records will be shown in the same bar +* Secondary axis (e.g. `y`): The height of the bars. If missing, it will be calculated by the layer * `colour`: The default colour of each bar * `stroke`: The colour of the stroke around each bar. Overrides `colour` * `fill`: The fill colour of each bar. Overrides `colour` @@ -27,7 +27,7 @@ The bar layer has no required aesthetics * `width`: The width of the bars as a proportion of the available width ## Data transformation -If `y` has not been mapped the layer will calculate it for you. +If secondary axis has not been mapped the layer will calculate it for you. ### Properties @@ -40,7 +40,10 @@ If `y` has not been mapped the layer will calculate it for you. ### Default remappings -* `count AS y`: By default the barplot will show count as the height of the bars +* `count AS `: By default the barplot will show count as the height of the bars + +## Orientation +Bar plots has categories along their primary axis. The orientation can be deduced purely from the mapping. To create a horizontal bar plot you map the categories to `y` instead of `x` (assuming a default Cartesian coordinate system). ## Examples @@ -87,3 +90,11 @@ DRAW bar SCALE BINNED x SETTING breaks => 10 ``` + +Create a horizontal bar plot by changing the mapping + +```{ggsql} +VISUALISE FROM ggsql:penguins +DRAW bar + MAPPING species AS y +``` diff --git a/doc/syntax/layer/boxplot.qmd b/doc/syntax/layer/boxplot.qmd index 10e8d2c4..52d6b0d5 100644 --- a/doc/syntax/layer/boxplot.qmd +++ b/doc/syntax/layer/boxplot.qmd @@ -9,8 +9,8 @@ Boxplots display a summary of a continuous distribution. In the style of Tukey, The following aesthetics are recognised by the boxplot layer. ### Required -* `x`: Position on the x-axis -* `y`: Position on the y-axis +* Primary axis (e.g. `x`): The categories to create boxplots for. +* Secondary axis (e.g. `y`): The values to calculate the summary statistics for the boxplot for ### Optional * `stroke`: The colour of the box contours, whiskers, median line and outliers. @@ -42,7 +42,10 @@ Observations are considered outliers when they are more extreme than the whisker ### Default remapping -* `value AS y`: By default the values are displayed along the y-axis. +* `value AS `: By default the values are displayed using the layers secondary axis. + +## Orientation +Boxplots has categories along their primary axis. The orientation can be deduced purely from the mapping. To create a boxplot with the boxplots "laying down" you map the categories to `y` instead of `x` (assuming a default Cartesian coordinate system). ### Examples diff --git a/doc/syntax/layer/density.qmd b/doc/syntax/layer/density.qmd index 97319e31..e4aacb23 100644 --- a/doc/syntax/layer/density.qmd +++ b/doc/syntax/layer/density.qmd @@ -10,7 +10,7 @@ Visualise the distribution of a single continuous variable by computing a kernel The following aesthetics are recognised by the density layer. ### Required -* `x`: Position on the x-axis. +* Primary axis (e.g. `x`): The variable to calculate density for. ### Optional * `stroke`: The colour of the contour lines. @@ -65,7 +65,10 @@ $$ ### Default remappings -* `density AS y`: By default the density layer will display the computed density along the y-axis. +* `density AS `: By default the density layer will display the computed density along its secondary axis. + +## Orientation +Density plots has the values to calculate the density of along their primary axis. The orientation can be deduced purely from the mapping. To create a vertical density plot you map the values to `y` instead of `x` (assuming a default Cartesian coordinate system). ## Examples diff --git a/doc/syntax/layer/histogram.qmd b/doc/syntax/layer/histogram.qmd index b0f57b79..abdbad4d 100644 --- a/doc/syntax/layer/histogram.qmd +++ b/doc/syntax/layer/histogram.qmd @@ -10,7 +10,7 @@ Visualise the distribution of a single continuous variable by dividing the x axi The following aesthetics are recognised by the bar layer. ### Required -* `x`: Position on the x-axis +* Primary axis (e.g. `x`): The value to bin and count. ### Optional * `colour`: The default colour of each bar @@ -27,7 +27,7 @@ The following aesthetics are recognised by the bar layer. * `closed`: Either `'left'` or `'right'` (default). Determines whether the bin intervals are closed to the left or right side ## Data transformation -The histogram layer will bin the records in each group and count them. By default it will map the count to `y`. +The histogram layer will bin the records in each group and count them. By default it will map the count to the secondary axis. ### Properties @@ -40,7 +40,10 @@ The histogram layer will bin the records in each group and count them. By defaul ### Default remappings -* `count AS y`: By default the histogram will show count as the height of the bars +* `count AS `: By default the histogram will show count as the height of the bars + +## Orientation +Histograms have the values to calculate to bin and count along their primary axis. The orientation can be deduced purely from the mapping. To create a vertical histogram you map the values to `y` instead of `x` (assuming a default Cartesian coordinate system). ## Examples diff --git a/doc/syntax/layer/line.qmd b/doc/syntax/layer/line.qmd index 6cbb6902..a40fe7df 100644 --- a/doc/syntax/layer/line.qmd +++ b/doc/syntax/layer/line.qmd @@ -10,8 +10,8 @@ The line layer is used to create lineplots. Lineplots always connects records al The following aesthetics are recognised by the line layer. ### Required -* `x`: Position along the x-axis -* `y`: Position along the y-axis +* Primary axis (e.g. `x`): The value of the independent variable. +* Secondary axis (e.g. `y`): The value of the dependent variable. ### Optional * `colour`/`stroke`: The colour of the line @@ -20,10 +20,13 @@ The following aesthetics are recognised by the line layer. * `linetype`: The type of line, i.e. the dashing pattern ## Settings -The line layer has no additional settings +* `orientation`: The orientation of the layer. Either `'aligned'` (default), or `'transposed'`. Determines if the layers primary axis is aligned with the coordinate systems first axis or not ## Data transformation -The line layer does not transform its data but passes it through unchanged +The line layer sorts the data along its primary axis + +## Orientation +Line plots are sorted and connected along their primary axis. Since the primary axis cannot be deduced from the mapping it must be specified using the `orientation` setting. E.g. if you wish to create a vertical line plot you need to set `orientation => 'transposed'` to indicate that the primary layer axis follows the second axis of the coordinate system. ## Examples diff --git a/doc/syntax/layer/path.qmd b/doc/syntax/layer/path.qmd index 439775ab..2fabd36b 100644 --- a/doc/syntax/layer/path.qmd +++ b/doc/syntax/layer/path.qmd @@ -10,8 +10,8 @@ The path layer is used to create lineplots, but contrary to the [line layer](lin The following aesthetics are recognised by the path layer. ### Required -* `x`: Position along the x-axis -* `y`: Position along the y-axis +* Primary axis (e.g. `x`): Position along the primary axis. +* Secondary axis (e.g. `y`): Position along the secondary axis. ### Optional * `colour`/`stroke`: The colour of the path @@ -20,10 +20,10 @@ The following aesthetics are recognised by the path layer. * `linetype`: The type of path, i.e. the dashing pattern ## Settings -The line layer has no additional settings +The path layer has no additional settings ## Data transformation -The line layer does not transform its data but passes it through unchanged +The path layer does not transform its data but passes it through unchanged ## Examples @@ -76,4 +76,4 @@ Compared to polygons, paths don't close their shapes and fill their interiors. VISUALISE x, y FROM df DRAW polygon MAPPING 'Polygon' AS stroke DRAW path MAPPING 'Path' AS stroke -``` \ No newline at end of file +``` diff --git a/doc/syntax/layer/point.qmd b/doc/syntax/layer/point.qmd index a1f181cb..0ad1c47a 100644 --- a/doc/syntax/layer/point.qmd +++ b/doc/syntax/layer/point.qmd @@ -10,8 +10,8 @@ The point layer is used to create scatterplots. The scatterplot is most useful f The following aesthetics are recognised by the point layer. ### Required -* `x`: Position along the x-axis -* `y`: Position along the y-axis +* Primary axis (e.g. `x`): Position along the primary axis. +* Secondary axis (e.g. `y`): Position along the secondary axis. ### Optional * `size`: The size of each point diff --git a/doc/syntax/layer/polygon.qmd b/doc/syntax/layer/polygon.qmd index ee9f3536..66328e42 100644 --- a/doc/syntax/layer/polygon.qmd +++ b/doc/syntax/layer/polygon.qmd @@ -10,8 +10,8 @@ Polygons can be used to draw arbitrary closed shapes based on an ordered sequenc The following aesthetics are recognised by the polygon layer. ### Required -* `x` Position along the x-axis. -* `y` Position along the y-axis. +* Primary axis (e.g. `x`): Position along the primary axis. +* Secondary axis (e.g. `y`): Position along the secondary axis. ### Optional * `stroke` The colour of the contour lines. @@ -62,4 +62,4 @@ Invoking a group through discrete aesthetics works as well. ```{ggsql} VISUALISE x, y FROM df DRAW polygon MAPPING id AS colour -``` \ No newline at end of file +``` diff --git a/doc/syntax/layer/ribbon.qmd b/doc/syntax/layer/ribbon.qmd index 2f87f88c..0ed8ad4f 100644 --- a/doc/syntax/layer/ribbon.qmd +++ b/doc/syntax/layer/ribbon.qmd @@ -10,9 +10,8 @@ The ribbon layer is used to display extrema over a sorted x-axis. It can be seen The following aesthetics are recognised by the ribbon layer. ### Required -* `x`: Position along the x-axis -* `ymin`: Lower position along the y-axis. -* `ymax`: Upper position along the y-axis. +* Primary axis (e.g. `x`): The value of the independent variable. +* Secondary axis range (e.g. `ymin` and `ymax`): The interval of the dependent variable. ### Optional * `stroke`: The colour of the contour lines. @@ -25,7 +24,10 @@ The following aesthetics are recognised by the ribbon layer. The ribbon layer has no additional settings. ## Data transformation -The ribbon layer does not transform its data but passes it through unchanged. +The line layer sorts the data along its primary position + +## Orientation +Ribbon layers are sorted and connected along their primary axis. The orientation can be deduced purely from the mapping since interval is mapped to the secodnary axis. To create a vertical ribbon layer you map the independent variable to `y` instead of `x` and the interval to `xmin` and `xmax` (assuming a default Cartesian coordinate system). ## Examples diff --git a/doc/syntax/layer/violin.qmd b/doc/syntax/layer/violin.qmd index 3283d9d2..372af2c7 100644 --- a/doc/syntax/layer/violin.qmd +++ b/doc/syntax/layer/violin.qmd @@ -11,8 +11,8 @@ The violins are mirrored kernel density estimates, similar to the [density](dens The following aesthetics are recognised by the violin layer. ### Required -* `x`: Position on the x-axis (categorical). -* `y`: Value on the y-axis for which to compute density. +* Primary axis (e.g. `x`): The categories to create violins for. +* Secondary axis (e.g. `y`): The values to calculate the density for the violins for ### Optional * `stroke`: The colour of the contour lines. @@ -50,6 +50,9 @@ The major difference between a violin layer and a density layer is just the matt * `density AS offset`: By default the offsets around a centerline reflect the computed density. +## Orientation +Violin plots has categories along their primary axis. The orientation can be deduced purely from the mapping. To create a violin plot with the violins "laying down" you map the categories to `y` instead of `x` (assuming a default Cartesian coordinate system). + ## Examples A typical violin plot. diff --git a/src/execute/layer.rs b/src/execute/layer.rs index d4d911fb..f497a4d8 100644 --- a/src/execute/layer.rs +++ b/src/execute/layer.rs @@ -110,20 +110,27 @@ pub fn build_layer_select_list( /// /// Note: Prefixed aesthetic names persist through the entire pipeline. /// We do NOT rename `__ggsql_aes_x__` back to `x`. +/// Apply remappings: rename stat columns and add literal columns. +/// +/// After stat transforms, columns like `__ggsql_stat_count` need to be renamed +/// to the target aesthetic's prefixed name (e.g., `__ggsql_aes_y__`). +/// For literal values (e.g., `ymin=0`), this creates a constant column. +/// +/// Note: Most column renames happen in the stat SQL query itself. +/// This handles any edge cases and all literal remappings. pub fn apply_remappings_post_query(df: DataFrame, layer: &Layer) -> Result { use polars::prelude::IntoColumn; let mut df = df; let row_count = df.height(); - // Apply remappings: stat columns → prefixed aesthetic names - // e.g., __ggsql_stat_count → __ggsql_aes_y__ - // Remappings structure: HashMap for (target_aesthetic, value) in &layer.remappings.aesthetics { let target_col_name = naming::aesthetic_column(target_aesthetic); match value { AestheticValue::Column { name, .. } => { + // Apply column remapping: stat columns → prefixed aesthetic names + // Note: Most renames happen in the stat SQL query itself // Check if this stat column exists in the DataFrame if df.column(name).is_ok() { df.rename(name, target_col_name.into()).map_err(|e| { @@ -135,17 +142,20 @@ pub fn apply_remappings_post_query(df: DataFrame, layer: &Layer) -> Result { - // Add constant column for literal values - let series = literal_to_series(&target_col_name, lit, row_count); - df = df - .with_column(series.into_column()) - .map_err(|e| { - GgsqlError::InternalError(format!( - "Failed to add literal column '{}': {}", - target_col_name, e - )) - })? - .clone(); + // Add literal column from remapping + // Only add if column doesn't already exist + if df.column(&target_col_name).is_err() { + let series = literal_to_series(&target_col_name, lit, row_count); + df = df + .with_column(series.into_column()) + .map_err(|e| { + GgsqlError::InternalError(format!( + "Failed to add literal column '{}': {}", + target_col_name, e + )) + })? + .clone(); + } } } } @@ -153,6 +163,7 @@ pub fn apply_remappings_post_query(df: DataFrame, layer: &Layer) -> Result polars::prelude::Series { use polars::prelude::{NamedFrom, Series}; @@ -337,6 +348,7 @@ pub fn build_layer_base_query( /// * `schema` - The layer's schema (with min/max from base_query) /// * `scales` - All resolved scales /// * `type_names` - SQL type names for the database backend +/// * `aesthetic_ctx` - Aesthetic context for name transformations /// * `execute_query` - Function to execute queries (needed for some stat transforms) /// /// # Returns @@ -348,15 +360,23 @@ pub fn apply_layer_transforms( schema: &Schema, scales: &[Scale], type_names: &SqlTypeNames, + aesthetic_ctx: &crate::plot::aesthetic::AestheticContext, execute_query: &F, ) -> Result where F: Fn(&str) -> Result, { + use crate::plot::layer::orientation::{flip_mappings, flip_remappings, Orientation}; + // Clone order_by early to avoid borrow conflicts let order_by = layer.order_by.clone(); + // Orientation detection and initial flip was already done in mod.rs before + // build_layer_base_query. We just check if we need to flip back after stat. + let needs_flip = layer.orientation == Some(Orientation::Transposed); + // Build the aesthetic-named schema for stat transforms + // Note: Mappings were already flipped in mod.rs if needed, so schema reflects normalized orientation let aesthetic_schema: Schema = build_aesthetic_schema(layer, schema); // Collect literal aesthetic column names BEFORE conversion to Column values. @@ -418,8 +438,17 @@ where execute_query, )?; + // Flip user remappings BEFORE merging defaults for Transposed orientation. + // User remappings are in user orientation (e.g., `count AS x` for horizontal histogram). + // We flip them to aligned orientation so they're uniform with defaults. + // At the end, we flip everything back together. + if needs_flip { + flip_remappings(layer); + } + // Apply literal default remappings from geom defaults (e.g., y2 => 0.0 for bar baseline). // These apply regardless of stat transform, but only if user hasn't overridden them. + // Defaults are always in aligned orientation. for (aesthetic, default_value) in layer.geom.default_remappings() { // Only process literal values here (Column values are handled in Transformed branch) if !matches!(default_value, DefaultAestheticValue::Column(_)) { @@ -555,7 +584,22 @@ where StatResult::Identity => query, }; - // Apply ORDER BY + // Flip mappings back after stat transforms if we flipped them earlier + // Now pos1/pos2 map to the user's intended x/y positions + // Note: We only flip mappings here, not remappings. Remappings are flipped + // later in mod.rs after apply_remappings_post_query creates the columns, + // so that Phase 4.5 can flip those columns along with everything else. + if needs_flip { + flip_mappings(layer); + + // Normalize mapping column names to match their aesthetic keys. + // After flip_mappings, pos1 might point to __ggsql_aes_pos2__ (and vice versa). + // We update the column names so pos1 → __ggsql_aes_pos1__, etc. + // The DataFrame columns will be renamed correspondingly in mod.rs. + normalize_mapping_column_names(layer, aesthetic_ctx); + } + + // Apply explicit ORDER BY if provided let final_query = if let Some(ref o) = order_by { format!("{} ORDER BY {}", final_query, o.as_str()) } else { @@ -564,3 +608,55 @@ where Ok(final_query) } + +/// Normalize mapping column names to match their aesthetic keys after flip-back. +/// +/// After `flip_mappings`, the mapping values (column names) may not match the keys. +/// For example, pos1 might point to `__ggsql_aes_pos2__`. +/// This function updates the column names so pos1 → `__ggsql_aes_pos1__`, etc. +/// +/// This should be called after flip_mappings during flip-back. +/// The DataFrame columns should be renamed correspondingly using `flip_dataframe_columns`. +fn normalize_mapping_column_names( + layer: &mut Layer, + aesthetic_ctx: &crate::plot::aesthetic::AestheticContext, +) { + // Collect the aesthetics to update (to avoid borrowing issues) + let aesthetics_to_update: Vec = layer + .mappings + .aesthetics + .keys() + .filter(|aes| crate::plot::aesthetic::is_positional_aesthetic(aes)) + .cloned() + .collect(); + + for aesthetic in aesthetics_to_update { + if let Some(value) = layer.mappings.aesthetics.get_mut(&aesthetic) { + let expected_col = naming::aesthetic_column(&aesthetic); + match value { + AestheticValue::Column { + name, original_name, .. + } => { + // The current column name might be wrong (e.g., __ggsql_aes_pos2__ for pos1) + // We need to flip it to match the aesthetic key + let current_col_aesthetic = + naming::extract_aesthetic_name(name).unwrap_or_default(); + let flipped_aesthetic = aesthetic_ctx.flip_positional(¤t_col_aesthetic); + + // If flipping results in the correct aesthetic, update the column name + if flipped_aesthetic == aesthetic { + *name = expected_col; + } + // Preserve original_name for axis labels + if original_name.is_none() { + *original_name = Some(aesthetic.clone()); + } + } + AestheticValue::Literal(_) => { + // Literals become columns with the expected name + *value = AestheticValue::standard_column(expected_col); + } + } + } + } +} diff --git a/src/execute/mod.rs b/src/execute/mod.rs index a8f17e07..7889b961 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -34,6 +34,84 @@ use crate::reader::Reader; #[cfg(all(feature = "duckdb", test))] use crate::reader::DuckDBReader; +// ============================================================================= +// DataFrame Column Flipping for Orientation +// ============================================================================= + +/// Flip positional column names in a DataFrame for Transposed orientation layers. +/// +/// Swaps column names like `__ggsql_aes_pos1__` ↔ `__ggsql_aes_pos2__` so that +/// the data matches the flipped mapping names. +/// +/// This is called after query execution for layers with Transposed orientation, +/// in coordination with `normalize_mapping_column_names` which updates the mappings. +fn flip_dataframe_positional_columns(df: DataFrame, aesthetic_ctx: &AestheticContext) -> DataFrame { + use polars::prelude::*; + + // Collect all positional column renames needed + let mut renames: Vec<(String, String)> = Vec::new(); + + for col_name in df.get_column_names() { + let col_str = col_name.to_string(); + + // Check if this is an aesthetic column (__ggsql_aes_XXX__) + if let Some(aesthetic) = naming::extract_aesthetic_name(&col_str) { + if is_positional_aesthetic(&aesthetic) { + let flipped = aesthetic_ctx.flip_positional(&aesthetic); + if flipped != aesthetic { + let new_col_name = naming::aesthetic_column(&flipped); + renames.push((col_str, new_col_name)); + } + } + } + } + + if renames.is_empty() { + return df; + } + + // To swap columns (A ↔ B), we need to: + // 1. Rename A to temp + // 2. Rename B to A + // 3. Rename temp to B + // But if both A and B exist, we need to handle them together + + let df_clone = df.clone(); // For fallback if collect fails + let mut lazy = df.lazy(); + + // Group renames into swap pairs + let mut processed: HashSet = HashSet::new(); + for (from, to) in &renames { + if processed.contains(from) { + continue; + } + + // Check if the reverse rename also exists (it's a swap) + let is_swap = renames.iter().any(|(f, t)| f == to && t == from); + + if is_swap { + // Swap: use intermediate names to avoid collision + let temp_from = format!("{}_temp_swap_", from); + let temp_to = format!("{}_temp_swap_", to); + + lazy = lazy + .rename([from.as_str()], [temp_from.as_str()], true) + .rename([to.as_str()], [temp_to.as_str()], true) + .rename([temp_from.as_str()], [to.as_str()], true) + .rename([temp_to.as_str()], [from.as_str()], true); + + processed.insert(from.clone()); + processed.insert(to.clone()); + } else { + // Simple rename (one direction only) + lazy = lazy.rename([from.as_str()], [to.as_str()], true); + processed.insert(from.clone()); + } + } + + lazy.collect().unwrap_or(df_clone) +} + // ============================================================================= // Validation // ============================================================================= @@ -1011,6 +1089,23 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result(query: &str, reader: &R) -> Result(query: &str, reader: &R) -> Result = HashMap::new(); + // Key by (query, serialized_remappings, orientation) to detect when layers can share data + // Layers with identical query AND remappings AND orientation share data via data_key + let mut config_to_key: HashMap<(String, String, bool), String> = HashMap::new(); for (idx, q) in layer_queries.iter().enumerate() { let layer = &mut specs[0].layers[idx]; let remappings_key = serde_json::to_string(&layer.remappings).unwrap_or_default(); - let config_key = (q.clone(), remappings_key); + let needs_flip = + layer.orientation == Some(crate::plot::layer::orientation::Orientation::Transposed); + let config_key = (q.clone(), remappings_key, needs_flip); if let Some(existing_key) = config_to_key.get(&config_key) { - // Same query AND same remappings - share data + // Same query AND same remappings AND same orientation - share data layer.data_key = Some(existing_key.clone()); } else { - // Need own data entry (either first occurrence or different remappings) + // Need own data entry (either first occurrence or different config) let layer_key = naming::layer_key(idx); let df = query_to_result.get(q).unwrap().clone(); + data_map.insert(layer_key.clone(), df); config_to_key.insert(config_key, layer_key.clone()); layer.data_key = Some(layer_key); } } - // Phase 4: Apply remappings (rename stat columns to prefixed aesthetic names) - // e.g., __ggsql_stat_count → __ggsql_aes_y__ + // Phase 4: Apply remappings (rename stat columns and add literal columns) + // e.g., __ggsql_stat_count → __ggsql_aes_y__, or add __ggsql_aes_pos2end__ = 0.0 // Note: Prefixed aesthetic names persist through the entire pipeline // Track processed keys to avoid duplicate work on shared datasets let mut processed_keys: HashSet = HashSet::new(); @@ -1128,6 +1227,18 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result(query: &str, reader: &R) -> Result = HashSet::new(); + for layer in specs[0].layers.iter() { + let needs_flip = + layer.orientation == Some(crate::plot::layer::orientation::Orientation::Transposed); + if needs_flip { + if let Some(ref key) = layer.data_key { + if flipped_keys.insert(key.clone()) { + // First time flipping this data key + if let Some(df) = data_map.remove(key) { + let flipped_df = + flip_dataframe_positional_columns(df, &aesthetic_ctx); + data_map.insert(key.clone(), flipped_df); + } + } + } + } + } + // Validate we have some data (every layer should have its own data) if data_map.is_empty() { return Err(GgsqlError::ValidationError( diff --git a/src/parser/builder.rs b/src/parser/builder.rs index f7bc9cb1..ec5b4678 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -477,6 +477,22 @@ fn build_layer(node: &Node, source: &SourceTree) -> Result { } } + // Extract orientation from parameters if present + let orientation = parameters + .remove("orientation") + .map(|v| { + match v { + ParameterValue::String(s) => s + .parse() + .map_err(|e: String| GgsqlError::ParseError(e)), + _ => Err(GgsqlError::ParseError( + "orientation must be a string ('aligned', 'transposed', 'vertical', or 'horizontal')" + .to_string(), + )), + } + }) + .transpose()?; + let mut layer = Layer::new(geom); layer.mappings = aesthetics; layer.remappings = remappings; @@ -485,6 +501,7 @@ fn build_layer(node: &Node, source: &SourceTree) -> Result { layer.filter = filter; layer.order_by = order_by; layer.source = layer_source; + layer.orientation = orientation; Ok(layer) } diff --git a/src/plot/aesthetic.rs b/src/plot/aesthetic.rs index b6253ed8..3635c645 100644 --- a/src/plot/aesthetic.rs +++ b/src/plot/aesthetic.rs @@ -30,6 +30,10 @@ use std::collections::HashMap; /// Positional aesthetic suffixes - applied to primary names to create variant aesthetics /// e.g., "x" + "min" = "xmin", "pos1" + "end" = "pos1end" +/// +/// Note: "offset" is intentionally NOT included here because it's a positioning +/// adjustment that shouldn't influence scale training or be part of aesthetic families. +/// The `flip_positional` method handles offset correctly via prefix detection. pub const POSITIONAL_SUFFIXES: &[&str] = &["min", "max", "end"]; // ============================================================================= @@ -304,6 +308,40 @@ impl AestheticContext { pub fn user_facet(&self) -> &[&'static str] { &self.user_facet } + + // === Orientation Flipping === + + /// Flip a positional aesthetic to its opposite position. + /// + /// Swaps pos1 ↔ pos2 (and their suffixed variants like pos1min ↔ pos2min). + /// Non-positional aesthetics are returned unchanged. + /// + /// # Examples + /// + /// ```ignore + /// let ctx = AestheticContext::from_static(&["x", "y"], &[]); + /// assert_eq!(ctx.flip_positional("pos1"), "pos2"); + /// assert_eq!(ctx.flip_positional("pos2min"), "pos1min"); + /// assert_eq!(ctx.flip_positional("pos1end"), "pos2end"); + /// assert_eq!(ctx.flip_positional("color"), "color"); // unchanged + /// ``` + pub fn flip_positional(&self, name: &str) -> String { + // Only flip if we have exactly 2 positional aesthetics + if self.internal_primaries.len() != 2 { + return name.to_string(); + } + + // Check if it's a pos1 or pos2 variant + if let Some(rest) = name.strip_prefix("pos1") { + return format!("pos2{}", rest); + } + if let Some(rest) = name.strip_prefix("pos2") { + return format!("pos1{}", rest); + } + + // Not a positional aesthetic, return unchanged + name.to_string() + } } /// Check if aesthetic is a user-facing facet aesthetic (panel, row, column) @@ -562,7 +600,7 @@ mod tests { fn test_aesthetic_context_families() { let ctx = AestheticContext::from_static(&["x", "y"], &[]); - // Get internal family + // Get internal family (offset not included - it's a positioning adjustment) let pos1_family = ctx.internal_positional_family("pos1").unwrap(); let pos1_strs: Vec<&str> = pos1_family.iter().map(|s| s.as_str()).collect(); assert_eq!(pos1_strs, vec!["pos1", "pos1min", "pos1max", "pos1end"]); diff --git a/src/plot/layer/geom/area.rs b/src/plot/layer/geom/area.rs index 06e8a7a0..0f340ad2 100644 --- a/src/plot/layer/geom/area.rs +++ b/src/plot/layer/geom/area.rs @@ -1,8 +1,9 @@ //! Area geom implementation use crate::plot::{types::DefaultAestheticValue, DefaultParam, DefaultParamValue}; +use crate::{naming, Mappings}; -use super::{DefaultAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType, StatResult}; /// Area geom - filled area charts #[derive(Debug, Clone, Copy)] @@ -33,6 +34,29 @@ impl GeomTrait for Area { default: DefaultParamValue::String("off"), }] } + + fn needs_stat_transform(&self, _aesthetics: &Mappings) -> bool { + true + } + + fn apply_stat_transform( + &self, + query: &str, + _schema: &crate::plot::Schema, + _aesthetics: &Mappings, + _group_by: &[String], + _parameters: &std::collections::HashMap, + _execute_query: &dyn Fn(&str) -> crate::Result, + ) -> crate::Result { + // Area geom needs ordering by pos1 (domain axis) for proper rendering + let order_col = naming::aesthetic_column("pos1"); + Ok(StatResult::Transformed { + query: format!("{} ORDER BY \"{}\"", query, order_col), + stat_columns: vec![], + dummy_columns: vec![], + consumed_aesthetics: vec![], + }) + } } impl std::fmt::Display for Area { diff --git a/src/plot/layer/geom/boxplot.rs b/src/plot/layer/geom/boxplot.rs index dda15397..5577e157 100644 --- a/src/plot/layer/geom/boxplot.rs +++ b/src/plot/layer/geom/boxplot.rs @@ -125,7 +125,8 @@ fn stat_boxplot( } }; - // Fix boxplots to be vertical, when we later have orientation this may change + // pos2 is the value axis, pos1 is the grouping axis (standard orientation) + // Orientation handling is done in the layer transform pipeline via flip_mappings() let (value_col, group_col) = (y, x); // The `groups` vector is never empty, it contains at least the opposite axis as column diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index e4a28d3d..2042350c 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -27,7 +27,9 @@ impl GeomTrait for Density { fn aesthetics(&self) -> DefaultAesthetics { DefaultAesthetics { defaults: &[ - ("pos1", DefaultAestheticValue::Required), + // pos_ means either pos1 or pos2 (orientation-agnostic) + // User maps x for vertical density, y for horizontal + ("pos_", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), @@ -68,6 +70,7 @@ impl GeomTrait for Density { &[ ("pos1", DefaultAestheticValue::Column("pos1")), ("pos2", DefaultAestheticValue::Column("density")), + ("pos2end", DefaultAestheticValue::Number(0.0)), // Baseline at 0 for area chart ] } diff --git a/src/plot/layer/geom/histogram.rs b/src/plot/layer/geom/histogram.rs index a94bf695..1a96c241 100644 --- a/src/plot/layer/geom/histogram.rs +++ b/src/plot/layer/geom/histogram.rs @@ -22,7 +22,9 @@ impl GeomTrait for Histogram { fn aesthetics(&self) -> DefaultAesthetics { DefaultAesthetics { defaults: &[ - ("pos1", DefaultAestheticValue::Required), + // pos_ means either pos1 or pos2 (orientation-agnostic) + // User maps x for vertical histogram, y for horizontal + ("pos_", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), diff --git a/src/plot/layer/geom/line.rs b/src/plot/layer/geom/line.rs index fa3dea59..53dfe962 100644 --- a/src/plot/layer/geom/line.rs +++ b/src/plot/layer/geom/line.rs @@ -1,7 +1,8 @@ //! Line geom implementation -use super::{DefaultAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType, StatResult}; use crate::plot::types::DefaultAestheticValue; +use crate::{naming, Mappings}; /// Line geom - line charts with connected points #[derive(Debug, Clone, Copy)] @@ -24,6 +25,29 @@ impl GeomTrait for Line { ], } } + + fn needs_stat_transform(&self, _aesthetics: &Mappings) -> bool { + true + } + + fn apply_stat_transform( + &self, + query: &str, + _schema: &crate::plot::Schema, + _aesthetics: &Mappings, + _group_by: &[String], + _parameters: &std::collections::HashMap, + _execute_query: &dyn Fn(&str) -> crate::Result, + ) -> crate::Result { + // Line geom needs ordering by pos1 (domain axis) for proper rendering + let order_col = naming::aesthetic_column("pos1"); + Ok(StatResult::Transformed { + query: format!("{} ORDER BY \"{}\"", query, order_col), + stat_columns: vec![], + dummy_columns: vec![], + consumed_aesthetics: vec![], + }) + } } impl std::fmt::Display for Line { diff --git a/src/plot/layer/geom/mod.rs b/src/plot/layer/geom/mod.rs index c953c9f7..a403fadc 100644 --- a/src/plot/layer/geom/mod.rs +++ b/src/plot/layer/geom/mod.rs @@ -209,11 +209,14 @@ pub trait GeomTrait: std::fmt::Debug + std::fmt::Display + Send + Sync { /// Returns valid parameter names for SETTING clause. /// /// Combines supported aesthetics with non-aesthetic parameters. + /// Includes "orientation" for all geoms (explicit override for auto-detection). fn valid_settings(&self) -> Vec<&'static str> { let mut valid: Vec<&'static str> = self.aesthetics().supported(); for param in self.default_params() { valid.push(param.name); } + // All geoms accept orientation parameter for explicit override + valid.push("orientation"); valid } } diff --git a/src/plot/layer/geom/ribbon.rs b/src/plot/layer/geom/ribbon.rs index 17777c9a..37ca967a 100644 --- a/src/plot/layer/geom/ribbon.rs +++ b/src/plot/layer/geom/ribbon.rs @@ -1,7 +1,8 @@ //! Ribbon geom implementation -use super::{DefaultAesthetics, GeomTrait, GeomType}; +use super::{DefaultAesthetics, GeomTrait, GeomType, StatResult}; use crate::plot::types::DefaultAestheticValue; +use crate::{naming, Mappings}; /// Ribbon geom - confidence bands and ranges #[derive(Debug, Clone, Copy)] @@ -26,6 +27,29 @@ impl GeomTrait for Ribbon { ], } } + + fn needs_stat_transform(&self, _aesthetics: &Mappings) -> bool { + true + } + + fn apply_stat_transform( + &self, + query: &str, + _schema: &crate::plot::Schema, + _aesthetics: &Mappings, + _group_by: &[String], + _parameters: &std::collections::HashMap, + _execute_query: &dyn Fn(&str) -> crate::Result, + ) -> crate::Result { + // Ribbon geom needs ordering by pos1 (domain axis) for proper rendering + let order_col = naming::aesthetic_column("pos1"); + Ok(StatResult::Transformed { + query: format!("{} ORDER BY \"{}\"", query, order_col), + stat_columns: vec![], + dummy_columns: vec![], + consumed_aesthetics: vec![], + }) + } } impl std::fmt::Display for Ribbon { diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index cb0632f4..0dd2d72a 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -27,17 +27,23 @@ impl DefaultAesthetics { } /// Get supported aesthetic names (excludes Delayed, for MAPPING validation) + /// + /// Note: `pos_` is expanded to both `pos1` and `pos2` for orientation-agnostic geoms. pub fn supported(&self) -> Vec<&'static str> { - self.defaults - .iter() - .filter_map(|(name, value)| { - if !matches!(value, DefaultAestheticValue::Delayed) { - Some(*name) - } else { - None - } - }) - .collect() + let mut result = Vec::new(); + for (name, value) in self.defaults { + if matches!(value, DefaultAestheticValue::Delayed) { + continue; + } + // Expand pos_ to both pos1 and pos2 + if *name == "pos_" { + result.push("pos1"); + result.push("pos2"); + } else { + result.push(*name); + } + } + result } /// Get required aesthetic names (those marked as Required) @@ -55,10 +61,19 @@ impl DefaultAesthetics { } /// Check if an aesthetic is supported (not Delayed) + /// + /// Note: If `pos_` is in defaults, both `pos1` and `pos2` are considered supported. pub fn is_supported(&self, name: &str) -> bool { - self.defaults - .iter() - .any(|(n, value)| *n == name && !matches!(value, DefaultAestheticValue::Delayed)) + self.defaults.iter().any(|(n, value)| { + if matches!(value, DefaultAestheticValue::Delayed) { + return false; + } + // pos_ supports both pos1 and pos2 + if *n == "pos_" && (name == "pos1" || name == "pos2") { + return true; + } + *n == name + }) } /// Check if an aesthetic exists (including Delayed) diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index 84076071..e2d653cd 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -8,6 +8,10 @@ use std::collections::HashMap; // Geom is now a submodule of layer pub mod geom; +pub mod orientation; + +// Re-export orientation types +pub use orientation::Orientation; // Re-export geom types for convenience pub use geom::{ @@ -55,6 +59,10 @@ pub struct Layer { pub order_by: Option, /// Columns for grouping/partitioning (from PARTITION BY clause) pub partition_by: Vec, + /// Layer orientation (None = auto-detect from scales) + /// Used for geoms with implicit orientation (bar, histogram, boxplot, etc.) + #[serde(skip_serializing_if = "Option::is_none")] + pub orientation: Option, /// Key for this layer's data in the datamap (set during execution). /// Defaults to `None`. Set to `__ggsql_layer___` during execution, /// but may point to another layer's data when queries are deduplicated. @@ -74,6 +82,7 @@ impl Layer { filter: None, order_by: None, partition_by: Vec::new(), + orientation: None, data_key: None, } } @@ -139,7 +148,15 @@ impl Layer { /// Check if this layer has the required aesthetics for its geom pub fn validate_required_aesthetics(&self) -> std::result::Result<(), String> { for aesthetic in self.geom.aesthetics().required() { - if !self.mappings.contains_key(aesthetic) { + // Special case: "pos_" means either pos1 or pos2 (orientation-agnostic) + if aesthetic == "pos_" { + if !self.mappings.contains_key("pos1") && !self.mappings.contains_key("pos2") { + return Err(format!( + "Geom '{}' requires a positional aesthetic (x or y) but none was provided", + self.geom + )); + } + } else if !self.mappings.contains_key(aesthetic) { return Err(format!( "Geom '{}' requires aesthetic '{}' but it was not provided", self.geom, aesthetic diff --git a/src/plot/layer/orientation.rs b/src/plot/layer/orientation.rs new file mode 100644 index 00000000..5769d86b --- /dev/null +++ b/src/plot/layer/orientation.rs @@ -0,0 +1,524 @@ +//! Layer orientation detection and mapping flipping. +//! +//! This module provides orientation detection for geoms with implicit orientation +//! (bar, histogram, boxplot, violin, density, ribbon) and handles flipping positional +//! aesthetic mappings before stat computation. +//! +//! # Orientation +//! +//! Some geoms have a "main axis" (categorical/domain axis) and a "value axis": +//! - Bar: main axis = categories, value axis = bar height +//! - Histogram: main axis = bins, value axis = count +//! - Boxplot: main axis = groups, value axis = distribution +//! - Ribbon: main axis = domain (e.g., time), value axis = range (min/max) +//! +//! Orientation describes how the layer's main axis aligns with the coordinate's +//! primary axis (pos1): +//! - **Aligned**: main axis = pos1 (vertical bars, x-axis bins) +//! - **Transposed**: main axis = pos2 (horizontal bars, y-axis bins) +//! +//! # Auto-Detection +//! +//! Orientation is auto-detected from scale types: +//! - For two-axis geoms (bar, boxplot): if pos1 is continuous and pos2 is discrete → Transposed +//! - For single-axis geoms (histogram, density): if pos2 has a scale but pos1 doesn't → Transposed +//! +//! # Explicit Override +//! +//! Users can set `SETTING orientation => 'transposed'` (or `'horizontal'`) or +//! `orientation => 'aligned'` (or `'vertical'`) to override auto-detection. + +use serde::{Deserialize, Serialize}; + +use super::geom::GeomType; +use super::Layer; +use crate::plot::scale::ScaleTypeKind; +use crate::plot::{Mappings, Scale}; + +/// Layer orientation for geoms with implicit direction. +/// +/// - **Aligned**: Layer's main axis aligns with coord's primary axis (pos1) +/// - **Transposed**: Layer's main axis aligns with coord's secondary axis (pos2) +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Orientation { + /// Aligned orientation: layer's main axis = pos1 (vertical bars, x-axis bins) + Aligned, + /// Transposed orientation: layer's main axis = pos2 (horizontal bars, y-axis bins) + Transposed, +} + +impl std::fmt::Display for Orientation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Orientation::Aligned => write!(f, "aligned"), + Orientation::Transposed => write!(f, "transposed"), + } + } +} + +impl std::str::FromStr for Orientation { + type Err = String; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "aligned" | "vertical" => Ok(Orientation::Aligned), + "transposed" | "horizontal" => Ok(Orientation::Transposed), + _ => Err(format!( + "Invalid orientation '{}'. Valid values: aligned, transposed, vertical, horizontal", + s + )), + } + } +} + +/// Determine effective orientation for a layer. +/// +/// If the layer has an explicit orientation set (from SETTING), use that. +/// Otherwise, auto-detect from scales for geoms with implicit orientation. +/// Geoms without implicit orientation always return Aligned. +pub fn resolve_orientation(layer: &Layer, scales: &[Scale]) -> Orientation { + // Explicit orientation always wins + if let Some(orientation) = layer.orientation { + return orientation; + } + + // Only auto-detect for geoms with implicit orientation + if !geom_has_implicit_orientation(&layer.geom.geom_type()) { + return Orientation::Aligned; + } + + detect_from_scales(scales, &layer.geom.geom_type(), &layer.mappings) +} + +/// Check if a geom type supports orientation auto-detection. +/// +/// Returns true for geoms with inherent orientation assumptions: +/// - Bar, Histogram, Boxplot, Violin, Density +/// +/// Returns false for geoms without inherent orientation: +/// - Point, Line, Path, Area, etc. +pub fn geom_has_implicit_orientation(geom: &GeomType) -> bool { + matches!( + geom, + GeomType::Bar + | GeomType::Histogram + | GeomType::Boxplot + | GeomType::Violin + | GeomType::Density + | GeomType::Ribbon + ) +} + +/// Detect orientation from scales and mappings. +/// +/// Applies unified rules in order: +/// +/// 1. **Single scale present**: The present scale defines the primary axis +/// - Only pos1 → Primary +/// - Only pos2 → Secondary +/// +/// 2. **Both continuous**: The axis with range mappings is secondary (value axis) +/// - pos1 has range mappings → Secondary +/// - pos2 has range mappings (or neither) → Primary (default) +/// +/// 3. **Mixed types**: The discrete scale is the primary (domain) axis +/// - pos1 discrete, pos2 continuous → Primary +/// - pos1 continuous, pos2 discrete → Secondary +/// +/// 4. **Default**: Primary +fn detect_from_scales(scales: &[Scale], _geom: &GeomType, mappings: &Mappings) -> Orientation { + let pos1_scale = scales.iter().find(|s| s.aesthetic == "pos1"); + let pos2_scale = scales.iter().find(|s| s.aesthetic == "pos2"); + + let has_pos1 = pos1_scale.is_some(); + let has_pos2 = pos2_scale.is_some(); + + // Rule 1: Single scale present - that axis is primary + if has_pos2 && !has_pos1 { + return Orientation::Transposed; + } + if has_pos1 && !has_pos2 { + return Orientation::Aligned; + } + + // Both scales present + let pos1_continuous = pos1_scale.is_some_and(is_continuous_scale); + let pos2_continuous = pos2_scale.is_some_and(is_continuous_scale); + + // Rule 2: Both continuous - range mapping axis is secondary + if pos1_continuous && pos2_continuous { + let has_pos1_range = + mappings.contains_key("pos1min") || mappings.contains_key("pos1max"); + let has_pos2_range = + mappings.contains_key("pos2min") || mappings.contains_key("pos2max"); + + if has_pos1_range && !has_pos2_range { + return Orientation::Transposed; + } + return Orientation::Aligned; + } + + // Rule 3: Mixed types - discrete axis is primary + let pos1_discrete = pos1_scale.is_some_and(is_discrete_scale); + let pos2_discrete = pos2_scale.is_some_and(is_discrete_scale); + + if pos1_continuous && pos2_discrete { + return Orientation::Transposed; + } + if pos1_discrete && pos2_continuous { + return Orientation::Aligned; + } + + // Default + Orientation::Aligned +} + +/// Check if a scale is continuous (numeric/temporal). +fn is_continuous_scale(scale: &Scale) -> bool { + scale + .scale_type + .as_ref() + .is_some_and(|st| st.scale_type_kind() == ScaleTypeKind::Continuous) +} + +/// Check if a scale is discrete (categorical/ordinal). +fn is_discrete_scale(scale: &Scale) -> bool { + scale.scale_type.as_ref().is_some_and(|st| { + matches!( + st.scale_type_kind(), + ScaleTypeKind::Discrete | ScaleTypeKind::Ordinal + ) + }) +} + +/// Swap positional aesthetic pairs in layer mappings. +/// +/// Swaps the following pairs: +/// - pos1 ↔ pos2 +/// - pos1min ↔ pos2min +/// - pos1max ↔ pos2max +/// - pos1end ↔ pos2end +/// - pos1offset ↔ pos2offset +/// +/// This is called before stat transforms for Transposed orientation layers, +/// so stats always see "aligned" orientation. After stat transforms, +/// this is called again to flip back to the correct output positions. +pub fn flip_mappings(layer: &mut Layer) { + let pairs = [ + ("pos1", "pos2"), + ("pos1min", "pos2min"), + ("pos1max", "pos2max"), + ("pos1end", "pos2end"), + ("pos1offset", "pos2offset"), + ]; + + for (a, b) in pairs { + let val_a = layer.mappings.aesthetics.remove(a); + let val_b = layer.mappings.aesthetics.remove(b); + + if let Some(v) = val_a { + layer.mappings.aesthetics.insert(b.to_string(), v); + } + if let Some(v) = val_b { + layer.mappings.aesthetics.insert(a.to_string(), v); + } + } +} + +/// Swap positional aesthetic pairs in remappings. +/// +/// Same as flip_mappings but for remappings (stat output mappings). +pub fn flip_remappings(layer: &mut Layer) { + let pairs = [ + ("pos1", "pos2"), + ("pos1min", "pos2min"), + ("pos1max", "pos2max"), + ("pos1end", "pos2end"), + ("pos1offset", "pos2offset"), + ]; + + for (a, b) in pairs { + let val_a = layer.remappings.aesthetics.remove(a); + let val_b = layer.remappings.aesthetics.remove(b); + + if let Some(v) = val_a { + layer.remappings.aesthetics.insert(b.to_string(), v); + } + if let Some(v) = val_b { + layer.remappings.aesthetics.insert(a.to_string(), v); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::plot::{AestheticValue, Geom, ScaleType}; + + #[test] + fn test_orientation_from_str() { + assert_eq!( + "aligned".parse::().unwrap(), + Orientation::Aligned + ); + assert_eq!( + "vertical".parse::().unwrap(), + Orientation::Aligned + ); + assert_eq!( + "transposed".parse::().unwrap(), + Orientation::Transposed + ); + assert_eq!( + "horizontal".parse::().unwrap(), + Orientation::Transposed + ); + assert_eq!( + "TRANSPOSED".parse::().unwrap(), + Orientation::Transposed + ); + assert!("invalid".parse::().is_err()); + } + + #[test] + fn test_orientation_display() { + assert_eq!(Orientation::Aligned.to_string(), "aligned"); + assert_eq!(Orientation::Transposed.to_string(), "transposed"); + } + + #[test] + fn test_geom_has_implicit_orientation() { + assert!(geom_has_implicit_orientation(&GeomType::Bar)); + assert!(geom_has_implicit_orientation(&GeomType::Histogram)); + assert!(geom_has_implicit_orientation(&GeomType::Boxplot)); + assert!(geom_has_implicit_orientation(&GeomType::Violin)); + assert!(geom_has_implicit_orientation(&GeomType::Density)); + assert!(geom_has_implicit_orientation(&GeomType::Ribbon)); + + assert!(!geom_has_implicit_orientation(&GeomType::Point)); + assert!(!geom_has_implicit_orientation(&GeomType::Line)); + assert!(!geom_has_implicit_orientation(&GeomType::Path)); + assert!(!geom_has_implicit_orientation(&GeomType::Area)); + } + + #[test] + fn test_resolve_orientation_explicit() { + let mut layer = Layer::new(Geom::bar()); + layer.orientation = Some(Orientation::Transposed); + + let scales = vec![]; + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Transposed); + } + + #[test] + fn test_resolve_orientation_no_implicit() { + // Point geom has no implicit orientation + let layer = Layer::new(Geom::point()); + let scales = vec![]; + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + + #[test] + fn test_resolve_orientation_histogram_default() { + // Histogram with pos1 scale → Aligned + let layer = Layer::new(Geom::histogram()); + let mut scale = Scale::new("pos1"); + scale.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + + #[test] + fn test_resolve_orientation_histogram_horizontal() { + // Histogram with only pos2 scale → Transposed + let layer = Layer::new(Geom::histogram()); + let mut scale = Scale::new("pos2"); + scale.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Transposed); + } + + #[test] + fn test_resolve_orientation_bar_horizontal() { + // Bar with pos1 continuous, pos2 discrete → Transposed + let layer = Layer::new(Geom::bar()); + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::continuous()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::discrete()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Transposed); + } + + #[test] + fn test_resolve_orientation_bar_vertical() { + // Bar with pos1 discrete, pos2 continuous → Aligned + let layer = Layer::new(Geom::bar()); + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::discrete()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + + #[test] + fn test_flip_mappings() { + let mut layer = Layer::new(Geom::bar()); + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("category".to_string()), + ); + layer.mappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("value".to_string()), + ); + layer.mappings.insert( + "pos1end".to_string(), + AestheticValue::standard_column("x2".to_string()), + ); + + flip_mappings(&mut layer); + + // pos1 ↔ pos2 + assert_eq!( + layer.mappings.get("pos2").unwrap().column_name(), + Some("category") + ); + assert_eq!( + layer.mappings.get("pos1").unwrap().column_name(), + Some("value") + ); + // pos1end → pos2end + assert_eq!( + layer.mappings.get("pos2end").unwrap().column_name(), + Some("x2") + ); + assert!(layer.mappings.get("pos1end").is_none()); + } + + #[test] + fn test_flip_mappings_empty() { + let mut layer = Layer::new(Geom::point()); + // No crash with empty mappings + flip_mappings(&mut layer); + assert!(layer.mappings.aesthetics.is_empty()); + } + + #[test] + fn test_flip_mappings_partial() { + let mut layer = Layer::new(Geom::bar()); + // Only pos1 mapped + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("x".to_string()), + ); + + flip_mappings(&mut layer); + + // pos1 moves to pos2 + assert!(layer.mappings.get("pos1").is_none()); + assert_eq!(layer.mappings.get("pos2").unwrap().column_name(), Some("x")); + } + + #[test] + fn test_resolve_orientation_ribbon_both_continuous_pos2_range() { + // Ribbon with both continuous scales and pos2 range → Aligned + let mut layer = Layer::new(Geom::ribbon()); + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("x".to_string()), + ); + layer.mappings.insert( + "pos2min".to_string(), + AestheticValue::standard_column("ymin".to_string()), + ); + layer.mappings.insert( + "pos2max".to_string(), + AestheticValue::standard_column("ymax".to_string()), + ); + + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::continuous()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + + #[test] + fn test_resolve_orientation_ribbon_both_continuous_pos1_range() { + // Ribbon with both continuous scales and pos1 range → Transposed + let mut layer = Layer::new(Geom::ribbon()); + layer.mappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("y".to_string()), + ); + layer.mappings.insert( + "pos1min".to_string(), + AestheticValue::standard_column("xmin".to_string()), + ); + layer.mappings.insert( + "pos1max".to_string(), + AestheticValue::standard_column("xmax".to_string()), + ); + + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::continuous()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Transposed); + } + + #[test] + fn test_resolve_orientation_ribbon_pos1_continuous_pos2_discrete() { + // Ribbon with pos1 continuous, pos2 discrete → Transposed + let mut layer = Layer::new(Geom::ribbon()); + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("value".to_string()), + ); + layer.mappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("category".to_string()), + ); + + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::continuous()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::discrete()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Transposed); + } + + #[test] + fn test_resolve_orientation_ribbon_pos1_discrete_pos2_continuous() { + // Ribbon with pos1 discrete, pos2 continuous → Aligned + let mut layer = Layer::new(Geom::ribbon()); + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("category".to_string()), + ); + layer.mappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("value".to_string()), + ); + + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::discrete()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } +} diff --git a/src/plot/main.rs b/src/plot/main.rs index a211ac34..dd2afd0c 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -495,9 +495,9 @@ mod tests { assert!(text.is_supported("family")); assert_eq!(text.required(), &["pos1", "pos2"]); - // Statistical geoms only require pos1 - assert_eq!(Geom::histogram().aesthetics().required(), &["pos1"]); - assert_eq!(Geom::density().aesthetics().required(), &["pos1"]); + // Statistical geoms require a positional aesthetic (pos_ = either pos1 or pos2) + assert_eq!(Geom::histogram().aesthetics().required(), &["pos_"]); + assert_eq!(Geom::density().aesthetics().required(), &["pos_"]); // Ribbon requires pos2min/pos2max assert_eq!( diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index 311b3381..83d6ab7c 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -131,6 +131,7 @@ pub trait GeomRenderer: Send + Sync { df: &DataFrame, _data_key: &str, binned_columns: &HashMap>, + _layer: &Layer, ) -> Result { let values = if binned_columns.is_empty() { dataframe_to_values(df)? @@ -194,15 +195,28 @@ pub struct BarRenderer; impl GeomRenderer for BarRenderer { fn modify_spec(&self, layer_spec: &mut Value, layer: &Layer) -> Result<()> { + use crate::plot::layer::Orientation; + let width = match layer.parameters.get("width") { Some(ParameterValue::Number(w)) => *w, _ => 0.9, }; - layer_spec["mark"] = json!({ - "type": "bar", - "width": {"band": width}, - "clip": true - }); + + // For horizontal bars, use "height" for band size; for vertical, use "width" + let is_horizontal = layer.orientation == Some(Orientation::Transposed); + layer_spec["mark"] = if is_horizontal { + json!({ + "type": "bar", + "height": {"band": width}, + "clip": true + }) + } else { + json!({ + "type": "bar", + "width": {"band": width}, + "clip": true + }) + }; Ok(()) } } @@ -222,11 +236,26 @@ impl GeomRenderer for PathRenderer { } } +// ============================================================================= +// Line Renderer +// ============================================================================= + +/// Renderer for line geom - preserves data order for correct line rendering +pub struct LineRenderer; + +impl GeomRenderer for LineRenderer { + fn modify_encoding(&self, encoding: &mut Map, _layer: &Layer) -> Result<()> { + // Use the data order (we've already ordered in SQL via apply_layer_transforms) + encoding.insert("order".to_string(), json!({"value": Value::Null})); + Ok(()) + } +} + // ============================================================================= // Ribbon Renderer // ============================================================================= -/// Renderer for ribbon geom - remaps ymin/ymax to y/y2 +/// Renderer for ribbon geom - remaps ymin/ymax to y/y2 and preserves data order pub struct RibbonRenderer; impl GeomRenderer for RibbonRenderer { @@ -237,6 +266,8 @@ impl GeomRenderer for RibbonRenderer { if let Some(ymin) = encoding.remove("ymin") { encoding.insert("y2".to_string(), ymin); } + // Use the data order (we've already ordered in SQL via apply_layer_transforms) + encoding.insert("order".to_string(), json!({"value": Value::Null})); Ok(()) } } @@ -311,7 +342,9 @@ impl GeomRenderer for PolygonRenderer { pub struct ViolinRenderer; impl GeomRenderer for ViolinRenderer { - fn modify_spec(&self, layer_spec: &mut Value, _layer: &Layer) -> Result<()> { + fn modify_spec(&self, layer_spec: &mut Value, layer: &Layer) -> Result<()> { + use crate::plot::layer::Orientation; + layer_spec["mark"] = json!({ "type": "line", "filled": true @@ -322,12 +355,24 @@ impl GeomRenderer for ViolinRenderer { // It'll be implemented as an offset. let violin_offset = format!("[datum.{offset}, -datum.{offset}]", offset = offset_col); + // Read orientation from layer (already resolved during execution) + let is_horizontal = layer.orientation == Some(Orientation::Transposed); + + // Continuous axis column for order calculation: + // - Vertical: pos2 (y-axis has continuous density values) + // - Horizontal: pos1 (x-axis has continuous density values) + let continuous_col = if is_horizontal { + naming::aesthetic_column("pos1") + } else { + naming::aesthetic_column("pos2") + }; + // We use an order calculation to create a proper closed shape. - // Right side (+ offset), sort by -y (top -> bottom) - // Left side (- offset), sort by +y (bottom -> top) + // Right side (+ offset), sort by -continuous (top -> bottom) + // Left side (- offset), sort by +continuous (bottom -> top) let calc_order = format!( - "datum.__violin_offset > 0 ? -datum.{y} : datum.{y}", - y = naming::aesthetic_column("pos2") + "datum.__violin_offset > 0 ? -datum.{} : datum.{}", + continuous_col, continuous_col ); // Filter threshold to trim very low density regions (removes thin tails) @@ -366,39 +411,49 @@ impl GeomRenderer for ViolinRenderer { Ok(()) } - fn modify_encoding(&self, encoding: &mut Map, _layer: &Layer) -> Result<()> { - // Ensure x is in detail encoding to create separate violins per x category + fn modify_encoding(&self, encoding: &mut Map, layer: &Layer) -> Result<()> { + use crate::plot::layer::Orientation; + + // Read orientation from layer (already resolved during execution) + let is_horizontal = layer.orientation == Some(Orientation::Transposed); + + // Categorical axis for detail encoding: + // - Vertical: x channel (categorical groups on x-axis) + // - Horizontal: y channel (categorical groups on y-axis) + let categorical_channel = if is_horizontal { "y" } else { "x" }; + + // Ensure categorical field is in detail encoding to create separate violins per category // This is needed because line marks with filled:true require detail to create separate paths - let x_field = encoding - .get("x") + let categorical_field = encoding + .get(categorical_channel) .and_then(|x| x.get("field")) .and_then(|f| f.as_str()) .map(|s| s.to_string()); - if let Some(x_field) = x_field { + if let Some(cat_field) = categorical_field { match encoding.get_mut("detail") { Some(detail) if detail.is_object() => { - // Single field object - check if it's already x, otherwise convert to array - if detail.get("field").and_then(|f| f.as_str()) != Some(&x_field) { + // Single field object - check if it's already the categorical field, otherwise convert to array + if detail.get("field").and_then(|f| f.as_str()) != Some(&cat_field) { let existing = detail.clone(); - *detail = json!([existing, {"field": x_field, "type": "nominal"}]); + *detail = json!([existing, {"field": cat_field, "type": "nominal"}]); } } Some(detail) if detail.is_array() => { - // Array - check if x already present, add if not + // Array - check if categorical field already present, add if not let arr = detail.as_array_mut().unwrap(); - let has_x = arr + let has_cat = arr .iter() - .any(|d| d.get("field").and_then(|f| f.as_str()) == Some(&x_field)); - if !has_x { - arr.push(json!({"field": x_field, "type": "nominal"})); + .any(|d| d.get("field").and_then(|f| f.as_str()) == Some(&cat_field)); + if !has_cat { + arr.push(json!({"field": cat_field, "type": "nominal"})); } } None => { - // No detail encoding - add it with x field + // No detail encoding - add it with categorical field encoding.insert( "detail".to_string(), - json!({"field": x_field, "type": "nominal"}), + json!({"field": cat_field, "type": "nominal"}), ); } _ => {} @@ -429,8 +484,12 @@ impl GeomRenderer for ViolinRenderer { } } + // Offset channel: + // - Vertical: xOffset (offsets left/right from category) + // - Horizontal: yOffset (offsets up/down from category) + let offset_channel = if is_horizontal { "yOffset" } else { "xOffset" }; encoding.insert( - "xOffset".to_string(), + offset_channel.to_string(), json!({ "field": "__violin_offset", "type": "quantitative" @@ -472,12 +531,26 @@ impl BoxplotRenderer { &self, data: &DataFrame, binned_columns: &HashMap>, + is_horizontal: bool, ) -> Result<(HashMap>, Vec, bool)> { let type_col = naming::aesthetic_column("type"); let type_col = type_col.as_str(); - let value_col = naming::aesthetic_column("pos2"); + + // Value columns depend on orientation (after DataFrame column flip): + // - Vertical: values in pos2/pos2end (no flip) + // - Horizontal: values in pos1/pos1end (was pos2/pos2end before flip) + let (value_col, value2_col) = if is_horizontal { + ( + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos1end"), + ) + } else { + ( + naming::aesthetic_column("pos2"), + naming::aesthetic_column("pos2end"), + ) + }; let value_col = value_col.as_str(); - let value2_col = naming::aesthetic_column("pos2end"); let value2_col = value2_col.as_str(); // Find grouping columns (all columns except type, value, value2) @@ -541,29 +614,40 @@ impl BoxplotRenderer { grouping_cols: &[String], has_outliers: bool, ) -> Result> { + use crate::plot::layer::Orientation; + let mut layers: Vec = Vec::new(); - let value_col = naming::aesthetic_column("pos2"); - let value2_col = naming::aesthetic_column("pos2end"); + // Use resolved orientation from layer (set during apply_layer_transforms) + let is_horizontal = layer.orientation == Some(Orientation::Transposed); + + // Value columns are predictable based on orientation: + // - Vertical: value in pos2, value2 in pos2end + // - Horizontal: value in pos1 (was pos2 before flip), value2 in pos1end + let (value_col, value2_col, group_aesthetic) = if is_horizontal { + ( + naming::aesthetic_column("pos1"), + naming::aesthetic_column("pos1end"), + "pos2", + ) + } else { + ( + naming::aesthetic_column("pos2"), + naming::aesthetic_column("pos2end"), + "pos1", + ) + }; - let x_col = layer + let group_col = layer .mappings - .get("pos1") + .get(group_aesthetic) .and_then(|x| x.column_name()) .ok_or_else(|| { - GgsqlError::WriterError("Boxplot requires 'x' aesthetic mapping".to_string()) + GgsqlError::WriterError(format!( + "Boxplot requires '{}' aesthetic mapping", + group_aesthetic + )) })?; - let y_col = layer - .mappings - .get("pos2") - .and_then(|y| y.column_name()) - .ok_or_else(|| { - GgsqlError::WriterError("Boxplot requires 'y' aesthetic mapping".to_string()) - })?; - - // Set orientation - let is_horizontal = x_col == value_col; - let group_col = if is_horizontal { y_col } else { x_col }; let offset = if is_horizontal { "yOffset" } else { "xOffset" }; let value_var1 = if is_horizontal { "x" } else { "y" }; let value_var2 = if is_horizontal { "x2" } else { "y2" }; @@ -683,28 +767,39 @@ impl BoxplotRenderer { upper_whiskers["encoding"][value_var2] = y2_encoding.clone(); // Box (bar from y to y2, where y=q1 and y2=q3) - let mut box_part = create_layer( - &summary_prototype, - "box", + // For horizontal boxplots, use "height" for band size; for vertical, use "width" + let box_mark = if is_horizontal { + json!({ + "type": "bar", + "height": {"band": width}, + "align": "center" + }) + } else { json!({ "type": "bar", "width": {"band": width}, "align": "center" - }), - ); + }) + }; + let mut box_part = create_layer(&summary_prototype, "box", box_mark); box_part["encoding"][value_var1] = y_encoding.clone(); box_part["encoding"][value_var2] = y2_encoding.clone(); // Median line (tick at y, where y=median) - let mut median_line = create_layer( - &summary_prototype, - "median", + let median_mark = if is_horizontal { + json!({ + "type": "tick", + "height": {"band": width}, + "align": "center" + }) + } else { json!({ "type": "tick", "width": {"band": width}, "align": "center" - }), - ); + }) + }; + let mut median_line = create_layer(&summary_prototype, "median", median_mark); median_line["encoding"][value_var1] = y_encoding; // Add dodging to all summary layers @@ -731,9 +826,13 @@ impl GeomRenderer for BoxplotRenderer { df: &DataFrame, _data_key: &str, binned_columns: &HashMap>, + layer: &Layer, ) -> Result { + use crate::plot::layer::Orientation; + let is_horizontal = layer.orientation == Some(Orientation::Transposed); + let (components, grouping_cols, has_outliers) = - self.prepare_components(df, binned_columns)?; + self.prepare_components(df, binned_columns, is_horizontal)?; Ok(PreparedData::Composite { components, @@ -783,6 +882,7 @@ impl GeomRenderer for BoxplotRenderer { /// Get the appropriate renderer for a geom type pub fn get_renderer(geom: &Geom) -> Box { match geom.geom_type() { + GeomType::Line => Box::new(LineRenderer), GeomType::Path => Box::new(PathRenderer), GeomType::Bar => Box::new(BarRenderer), GeomType::Area => Box::new(AreaRenderer), @@ -791,7 +891,7 @@ pub fn get_renderer(geom: &Geom) -> Box { GeomType::Boxplot => Box::new(BoxplotRenderer), GeomType::Density => Box::new(AreaRenderer), GeomType::Violin => Box::new(ViolinRenderer), - // All other geoms (Point, Line, Tile, etc.) use the default renderer + // All other geoms (Point, Tile, etc.) use the default renderer _ => Box::new(DefaultRenderer), } } diff --git a/src/writer/vegalite/mod.rs b/src/writer/vegalite/mod.rs index 7e5c7ef6..5698a5cc 100644 --- a/src/writer/vegalite/mod.rs +++ b/src/writer/vegalite/mod.rs @@ -97,7 +97,7 @@ fn prepare_layer_data( let renderer = get_renderer(&layer.geom); // Prepare data using the renderer (handles both standard and composite cases) - let prepared = renderer.prepare_data(df, data_key, binned_columns)?; + let prepared = renderer.prepare_data(df, data_key, binned_columns, layer)?; // Add data to individual datasets based on prepared type match &prepared { From 55223dc001dcd00cba9f462afb1fc664b486122f Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Mon, 9 Mar 2026 13:12:31 +0100 Subject: [PATCH 2/5] Add no mapping rule --- src/plot/layer/orientation.rs | 106 +++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/src/plot/layer/orientation.rs b/src/plot/layer/orientation.rs index 5769d86b..03abe042 100644 --- a/src/plot/layer/orientation.rs +++ b/src/plot/layer/orientation.rs @@ -88,7 +88,12 @@ pub fn resolve_orientation(layer: &Layer, scales: &[Scale]) -> Orientation { return Orientation::Aligned; } - detect_from_scales(scales, &layer.geom.geom_type(), &layer.mappings) + detect_from_scales( + scales, + &layer.geom.geom_type(), + &layer.mappings, + &layer.remappings, + ) } /// Check if a geom type supports orientation auto-detection. @@ -110,10 +115,15 @@ pub fn geom_has_implicit_orientation(geom: &GeomType) -> bool { ) } -/// Detect orientation from scales and mappings. +/// Detect orientation from scales, mappings, and remappings. /// /// Applies unified rules in order: /// +/// 0. **Remapping without mapping**: If no positional mappings exist but remappings +/// target a positional axis, the remapping target is the value axis: +/// - Remapping to pos1 only → Transposed (pos1 is value axis, main axis must be pos2) +/// - Remapping to pos2 only → Aligned (pos2 is value axis, main axis is pos1) +/// /// 1. **Single scale present**: The present scale defines the primary axis /// - Only pos1 → Primary /// - Only pos2 → Secondary @@ -127,7 +137,29 @@ pub fn geom_has_implicit_orientation(geom: &GeomType) -> bool { /// - pos1 continuous, pos2 discrete → Secondary /// /// 4. **Default**: Primary -fn detect_from_scales(scales: &[Scale], _geom: &GeomType, mappings: &Mappings) -> Orientation { +fn detect_from_scales( + scales: &[Scale], + _geom: &GeomType, + mappings: &Mappings, + remappings: &Mappings, +) -> Orientation { + // Check for positional mappings + let has_pos1_mapping = mappings.contains_key("pos1"); + let has_pos2_mapping = mappings.contains_key("pos2"); + + // Rule 0: Remapping without mapping - remapping target is the value axis + if !has_pos1_mapping && !has_pos2_mapping { + let has_pos1_remapping = remappings.contains_key("pos1"); + let has_pos2_remapping = remappings.contains_key("pos2"); + + if has_pos1_remapping && !has_pos2_remapping { + return Orientation::Transposed; + } + if has_pos2_remapping && !has_pos1_remapping { + return Orientation::Aligned; + } + } + let pos1_scale = scales.iter().find(|s| s.aesthetic == "pos1"); let pos2_scale = scales.iter().find(|s| s.aesthetic == "pos2"); @@ -521,4 +553,72 @@ mod tests { assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); } + + #[test] + fn test_resolve_orientation_remapping_to_pos1() { + // Bar with no mappings but remapping to pos1 → Transposed + // This covers: VISUALISE FROM data DRAW bar REMAPPING proportion AS x + let mut layer = Layer::new(Geom::bar()); + layer.remappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("proportion".to_string()), + ); + + let scales = vec![]; + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Transposed); + } + + #[test] + fn test_resolve_orientation_remapping_to_pos2() { + // Bar with no mappings but remapping to pos2 → Aligned (default) + let mut layer = Layer::new(Geom::bar()); + layer.remappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("count".to_string()), + ); + + let scales = vec![]; + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + + #[test] + fn test_resolve_orientation_remapping_both_axes() { + // Bar with remappings to both axes → falls through to default (Aligned) + let mut layer = Layer::new(Geom::bar()); + layer.remappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("x_val".to_string()), + ); + layer.remappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("y_val".to_string()), + ); + + let scales = vec![]; + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + + #[test] + fn test_resolve_orientation_mapping_overrides_remapping() { + // Bar with pos1 mapping AND pos1 remapping → mapping takes precedence + // The remapping rule only applies when NO positional mappings exist + let mut layer = Layer::new(Geom::bar()); + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("category".to_string()), + ); + layer.remappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("proportion".to_string()), + ); + + // With pos1 discrete scale → Aligned (normal rule 3) + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::discrete()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } } From 4101c79ac45db27fe40158f5f44b198211c3aed7 Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Mon, 9 Mar 2026 13:43:52 +0100 Subject: [PATCH 3/5] format and fix from clippy --- src/execute/layer.rs | 7 ++++--- src/execute/mod.rs | 7 +++---- src/plot/layer/orientation.rs | 36 +++++++++++++++++++++++++---------- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/execute/layer.rs b/src/execute/layer.rs index f497a4d8..c8092df3 100644 --- a/src/execute/layer.rs +++ b/src/execute/layer.rs @@ -163,7 +163,6 @@ pub fn apply_remappings_post_query(df: DataFrame, layer: &Layer) -> Result polars::prelude::Series { use polars::prelude::{NamedFrom, Series}; @@ -635,13 +634,15 @@ fn normalize_mapping_column_names( let expected_col = naming::aesthetic_column(&aesthetic); match value { AestheticValue::Column { - name, original_name, .. + name, + original_name, + .. } => { // The current column name might be wrong (e.g., __ggsql_aes_pos2__ for pos1) // We need to flip it to match the aesthetic key let current_col_aesthetic = naming::extract_aesthetic_name(name).unwrap_or_default(); - let flipped_aesthetic = aesthetic_ctx.flip_positional(¤t_col_aesthetic); + let flipped_aesthetic = aesthetic_ctx.flip_positional(current_col_aesthetic); // If flipping results in the correct aesthetic, update the column name if flipped_aesthetic == aesthetic { diff --git a/src/execute/mod.rs b/src/execute/mod.rs index 7889b961..e21d1589 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -56,8 +56,8 @@ fn flip_dataframe_positional_columns(df: DataFrame, aesthetic_ctx: &AestheticCon // Check if this is an aesthetic column (__ggsql_aes_XXX__) if let Some(aesthetic) = naming::extract_aesthetic_name(&col_str) { - if is_positional_aesthetic(&aesthetic) { - let flipped = aesthetic_ctx.flip_positional(&aesthetic); + if is_positional_aesthetic(aesthetic) { + let flipped = aesthetic_ctx.flip_positional(aesthetic); if flipped != aesthetic { let new_col_name = naming::aesthetic_column(&flipped); renames.push((col_str, new_col_name)); @@ -1262,8 +1262,7 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result Date: Tue, 10 Mar 2026 14:50:37 +0100 Subject: [PATCH 4/5] various fixes --- src/execute/mod.rs | 14 +++- src/plot/aesthetic.rs | 69 +++++++++++++++++ src/plot/layer/geom/density.rs | 5 +- src/plot/layer/geom/histogram.rs | 5 +- src/plot/layer/geom/types.rs | 55 +++++++------- src/plot/layer/mod.rs | 60 ++++++++++++--- src/plot/layer/orientation.rs | 80 ++++++++++++++++++-- src/plot/main.rs | 6 +- src/reader/data.rs | 125 +++++++++++++++++++++++++++++++ src/writer/vegalite/layer.rs | 31 +++++--- 10 files changed, 386 insertions(+), 64 deletions(-) diff --git a/src/execute/mod.rs b/src/execute/mod.rs index e21d1589..12d8052b 100644 --- a/src/execute/mod.rs +++ b/src/execute/mod.rs @@ -1097,11 +1097,23 @@ pub fn prepare_data_with_reader(query: &str, reader: &R) -> Result bool { false } +/// Parse a positional aesthetic name to extract its slot number and suffix. +/// +/// Returns `Some((slot, suffix))` for positional aesthetics: +/// - `pos1` → (1, "") +/// - `pos2min` → (2, "min") +/// - `pos1end` → (1, "end") +/// +/// Returns `None` for non-positional aesthetics. +pub fn parse_positional(name: &str) -> Option<(u8, &str)> { + if !name.starts_with("pos") { + return None; + } + let rest = &name[3..]; + let slot_char = rest.chars().next()?; + let slot = slot_char.to_digit(10)? as u8; + let suffix = &rest[1..]; + Some((slot, suffix)) +} + +/// Remap a positional aesthetic to a different slot. +/// +/// E.g., `remap_positional_slot("pos1min", 2)` → "pos2min" +pub fn remap_positional_slot(name: &str, new_slot: u8) -> Option { + let (_, suffix) = parse_positional(name)?; + Some(format!("pos{}{}", new_slot, suffix)) +} + #[cfg(test)] mod tests { use super::*; @@ -611,4 +638,46 @@ mod tests { assert_eq!(ctx.primary_internal_positional("pos2end"), Some("pos2")); assert_eq!(ctx.primary_internal_positional("color"), Some("color")); } + + #[test] + fn test_parse_positional() { + // Primary positional + assert_eq!(parse_positional("pos1"), Some((1, ""))); + assert_eq!(parse_positional("pos2"), Some((2, ""))); + + // Variants with suffixes + assert_eq!(parse_positional("pos1min"), Some((1, "min"))); + assert_eq!(parse_positional("pos2max"), Some((2, "max"))); + assert_eq!(parse_positional("pos1end"), Some((1, "end"))); + + // Non-positional + assert_eq!(parse_positional("color"), None); + assert_eq!(parse_positional("x"), None); + assert_eq!(parse_positional("xmin"), None); + } + + #[test] + fn test_remap_positional_slot() { + // Remap slot 1 → slot 2 + assert_eq!(remap_positional_slot("pos1", 2), Some("pos2".to_string())); + assert_eq!( + remap_positional_slot("pos1min", 2), + Some("pos2min".to_string()) + ); + assert_eq!( + remap_positional_slot("pos1end", 2), + Some("pos2end".to_string()) + ); + + // Remap slot 2 → slot 1 + assert_eq!(remap_positional_slot("pos2", 1), Some("pos1".to_string())); + assert_eq!( + remap_positional_slot("pos2max", 1), + Some("pos1max".to_string()) + ); + + // Non-positional returns None + assert_eq!(remap_positional_slot("color", 1), None); + assert_eq!(remap_positional_slot("x", 2), None); + } } diff --git a/src/plot/layer/geom/density.rs b/src/plot/layer/geom/density.rs index 2042350c..87cd4d31 100644 --- a/src/plot/layer/geom/density.rs +++ b/src/plot/layer/geom/density.rs @@ -27,9 +27,8 @@ impl GeomTrait for Density { fn aesthetics(&self) -> DefaultAesthetics { DefaultAesthetics { defaults: &[ - // pos_ means either pos1 or pos2 (orientation-agnostic) - // User maps x for vertical density, y for horizontal - ("pos_", DefaultAestheticValue::Required), + // pos1 for values to estimate density of (bidirectional: accepts pos1 or pos2) + ("pos1", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), diff --git a/src/plot/layer/geom/histogram.rs b/src/plot/layer/geom/histogram.rs index 1a96c241..eba244a1 100644 --- a/src/plot/layer/geom/histogram.rs +++ b/src/plot/layer/geom/histogram.rs @@ -22,9 +22,8 @@ impl GeomTrait for Histogram { fn aesthetics(&self) -> DefaultAesthetics { DefaultAesthetics { defaults: &[ - // pos_ means either pos1 or pos2 (orientation-agnostic) - // User maps x for vertical histogram, y for horizontal - ("pos_", DefaultAestheticValue::Required), + // pos1 for values to bin (bidirectional: accepts pos1 or pos2) + ("pos1", DefaultAestheticValue::Required), ("weight", DefaultAestheticValue::Null), ("fill", DefaultAestheticValue::String("black")), ("stroke", DefaultAestheticValue::String("black")), diff --git a/src/plot/layer/geom/types.rs b/src/plot/layer/geom/types.rs index 0dd2d72a..5e04f386 100644 --- a/src/plot/layer/geom/types.rs +++ b/src/plot/layer/geom/types.rs @@ -2,6 +2,7 @@ //! //! These types are used by all geom implementations and are shared across the module. +use crate::plot::aesthetic::parse_positional; use crate::{plot::types::DefaultAestheticValue, Mappings}; // Re-export shared types from the central location @@ -28,22 +29,14 @@ impl DefaultAesthetics { /// Get supported aesthetic names (excludes Delayed, for MAPPING validation) /// - /// Note: `pos_` is expanded to both `pos1` and `pos2` for orientation-agnostic geoms. + /// Returns the literal names from defaults. For bidirectional positional checking, + /// use `is_supported()` which handles pos1/pos2 equivalence. pub fn supported(&self) -> Vec<&'static str> { - let mut result = Vec::new(); - for (name, value) in self.defaults { - if matches!(value, DefaultAestheticValue::Delayed) { - continue; - } - // Expand pos_ to both pos1 and pos2 - if *name == "pos_" { - result.push("pos1"); - result.push("pos2"); - } else { - result.push(*name); - } - } - result + self.defaults + .iter() + .filter(|(_, value)| !matches!(value, DefaultAestheticValue::Delayed)) + .map(|(name, _)| *name) + .collect() } /// Get required aesthetic names (those marked as Required) @@ -62,18 +55,28 @@ impl DefaultAesthetics { /// Check if an aesthetic is supported (not Delayed) /// - /// Note: If `pos_` is in defaults, both `pos1` and `pos2` are considered supported. + /// Positional aesthetics are bidirectional: if pos1* is supported, pos2* is also + /// considered supported (and vice versa). pub fn is_supported(&self, name: &str) -> bool { - self.defaults.iter().any(|(n, value)| { - if matches!(value, DefaultAestheticValue::Delayed) { - return false; - } - // pos_ supports both pos1 and pos2 - if *n == "pos_" && (name == "pos1" || name == "pos2") { - return true; - } - *n == name - }) + // Check for direct match first + let direct_match = self + .defaults + .iter() + .any(|(n, value)| !matches!(value, DefaultAestheticValue::Delayed) && *n == name); + if direct_match { + return true; + } + + // Check for bidirectional positional match + if let Some((slot, suffix)) = parse_positional(name) { + let other_slot = if slot == 1 { 2 } else { 1 }; + let equivalent = format!("pos{}{}", other_slot, suffix); + return self.defaults.iter().any(|(n, value)| { + !matches!(value, DefaultAestheticValue::Delayed) && *n == equivalent + }); + } + + false } /// Check if an aesthetic exists (including Delayed) diff --git a/src/plot/layer/mod.rs b/src/plot/layer/mod.rs index e2d653cd..37ae1dc2 100644 --- a/src/plot/layer/mod.rs +++ b/src/plot/layer/mod.rs @@ -146,17 +146,32 @@ impl Layer { } /// Check if this layer has the required aesthetics for its geom + /// + /// Positional aesthetics (pos1*, pos2*) are validated bidirectionally: + /// requirements using pos1/pos2 slots can be satisfied by either axis assignment. + /// For example, requiring `pos1`, `pos2min`, `pos2max` accepts either: + /// - `x`, `ymin`, `ymax` (slot1→x-axis, slot2→y-axis) + /// - `y`, `xmin`, `xmax` (slot1→y-axis, slot2→x-axis) pub fn validate_required_aesthetics(&self) -> std::result::Result<(), String> { - for aesthetic in self.geom.aesthetics().required() { - // Special case: "pos_" means either pos1 or pos2 (orientation-agnostic) - if aesthetic == "pos_" { - if !self.mappings.contains_key("pos1") && !self.mappings.contains_key("pos2") { - return Err(format!( - "Geom '{}' requires a positional aesthetic (x or y) but none was provided", - self.geom - )); - } - } else if !self.mappings.contains_key(aesthetic) { + use crate::plot::aesthetic::parse_positional; + + let required = self.geom.aesthetics().required(); + + // Separate positional (with parsed slot/suffix) and non-positional requirements + let mut positional_reqs: Vec<(&str, u8, &str)> = Vec::new(); // (name, slot, suffix) + let mut other_reqs: Vec<&str> = Vec::new(); + + for aesthetic in &required { + if let Some((slot, suffix)) = parse_positional(aesthetic) { + positional_reqs.push((aesthetic, slot, suffix)); + } else { + other_reqs.push(aesthetic); + } + } + + // Validate non-positional requirements directly + for aesthetic in &other_reqs { + if !self.mappings.contains_key(aesthetic) { return Err(format!( "Geom '{}' requires aesthetic '{}' but it was not provided", self.geom, aesthetic @@ -164,6 +179,31 @@ impl Layer { } } + // Validate positional requirements bidirectionally + // Try both slot assignments: (1→1, 2→2) and (1→2, 2→1) + if !positional_reqs.is_empty() { + let identity_ok = positional_reqs + .iter() + .all(|(name, _, _)| self.mappings.contains_key(name)); + + let swapped_ok = positional_reqs.iter().all(|(_, slot, suffix)| { + let new_slot = if *slot == 1 { 2 } else { 1 }; + let remapped = format!("pos{}{}", new_slot, suffix); + self.mappings.contains_key(&remapped) + }); + + if !identity_ok && !swapped_ok { + let (missing, _, _) = positional_reqs + .iter() + .find(|(name, _, _)| !self.mappings.contains_key(name)) + .unwrap(); + return Err(format!( + "Geom '{}' requires aesthetic '{}' (or its bidirectional equivalent) but it was not provided", + self.geom, missing + )); + } + } + Ok(()) } diff --git a/src/plot/layer/orientation.rs b/src/plot/layer/orientation.rs index 090e3f94..272ee387 100644 --- a/src/plot/layer/orientation.rs +++ b/src/plot/layer/orientation.rs @@ -179,9 +179,14 @@ fn detect_from_scales( let pos2_continuous = pos2_scale.is_some_and(is_continuous_scale); // Rule 2: Both continuous - range mapping axis is secondary + // Range mappings include min/max pairs and primary/end pairs if pos1_continuous && pos2_continuous { - let has_pos1_range = mappings.contains_key("pos1min") || mappings.contains_key("pos1max"); - let has_pos2_range = mappings.contains_key("pos2min") || mappings.contains_key("pos2max"); + let has_pos1_range = mappings.contains_key("pos1min") + || mappings.contains_key("pos1max") + || mappings.contains_key("pos1end"); + let has_pos2_range = mappings.contains_key("pos2min") + || mappings.contains_key("pos2max") + || mappings.contains_key("pos2end"); if has_pos1_range && !has_pos2_range { return Orientation::Transposed; @@ -231,8 +236,8 @@ fn is_discrete_scale(scale: &Scale) -> bool { /// - pos1end ↔ pos2end /// - pos1offset ↔ pos2offset /// -/// This is called before stat transforms for Transposed orientation layers, -/// so stats always see "aligned" orientation. After stat transforms, +/// This is called before stat transforms for Secondary orientation layers, +/// so stats always see "standard" orientation. After stat transforms, /// this is called again to flip back to the correct output positions. pub fn flip_mappings(layer: &mut Layer) { let pairs = [ @@ -493,7 +498,7 @@ mod tests { #[test] fn test_resolve_orientation_ribbon_both_continuous_pos1_range() { - // Ribbon with both continuous scales and pos1 range → Transposed + // Ribbon with both continuous scales and pos1 range → Secondary let mut layer = Layer::new(Geom::ribbon()); layer.mappings.insert( "pos2".to_string(), @@ -522,7 +527,7 @@ mod tests { #[test] fn test_resolve_orientation_ribbon_pos1_continuous_pos2_discrete() { - // Ribbon with pos1 continuous, pos2 discrete → Transposed + // Ribbon with pos1 continuous, pos2 discrete → Secondary let mut layer = Layer::new(Geom::ribbon()); layer.mappings.insert( "pos1".to_string(), @@ -547,7 +552,7 @@ mod tests { #[test] fn test_resolve_orientation_ribbon_pos1_discrete_pos2_continuous() { - // Ribbon with pos1 discrete, pos2 continuous → Aligned + // Ribbon with pos1 discrete, pos2 continuous → Primary let mut layer = Layer::new(Geom::ribbon()); layer.mappings.insert( "pos1".to_string(), @@ -567,6 +572,67 @@ mod tests { assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); } + #[test] + fn test_resolve_orientation_ribbon_pos1_range_with_scales() { + // Ribbon with pos2 mapping and pos1 range (xmin/xmax) with continuous scales → Transposed + // This covers: DRAW ribbon MAPPING Date AS y, Temp AS xmax, 0.0 AS xmin + // Rule 2: Both continuous, pos1 has range → Transposed + let mut layer = Layer::new(Geom::ribbon()); + layer.mappings.insert( + "pos2".to_string(), + AestheticValue::standard_column("Date".to_string()), + ); + layer.mappings.insert( + "pos1min".to_string(), + AestheticValue::Literal(crate::plot::ParameterValue::Number(0.0)), + ); + layer.mappings.insert( + "pos1max".to_string(), + AestheticValue::standard_column("Temp".to_string()), + ); + + // Scales are created and typed by execute pipeline + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::continuous()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!( + resolve_orientation(&layer, &scales), + Orientation::Transposed + ); + } + + #[test] + fn test_resolve_orientation_ribbon_pos2_range_with_scales() { + // Ribbon with pos1 mapping and pos2 range (ymin/ymax) with continuous scales → Aligned + // This covers: DRAW ribbon MAPPING Date AS x, Temp AS ymax, 0.0 AS ymin + // Rule 2: Both continuous, pos2 has range (or neither) → Aligned + let mut layer = Layer::new(Geom::ribbon()); + layer.mappings.insert( + "pos1".to_string(), + AestheticValue::standard_column("Date".to_string()), + ); + layer.mappings.insert( + "pos2min".to_string(), + AestheticValue::Literal(crate::plot::ParameterValue::Number(0.0)), + ); + layer.mappings.insert( + "pos2max".to_string(), + AestheticValue::standard_column("Temp".to_string()), + ); + + // Scales are created and typed by execute pipeline + let mut scale1 = Scale::new("pos1"); + scale1.scale_type = Some(ScaleType::continuous()); + let mut scale2 = Scale::new("pos2"); + scale2.scale_type = Some(ScaleType::continuous()); + let scales = vec![scale1, scale2]; + + assert_eq!(resolve_orientation(&layer, &scales), Orientation::Aligned); + } + #[test] fn test_resolve_orientation_remapping_to_pos1() { // Bar with no mappings but remapping to pos1 → Transposed diff --git a/src/plot/main.rs b/src/plot/main.rs index 33a3f249..affedc65 100644 --- a/src/plot/main.rs +++ b/src/plot/main.rs @@ -495,9 +495,9 @@ mod tests { assert!(text.is_supported("family")); assert_eq!(text.required(), &["pos1", "pos2"]); - // Statistical geoms require a positional aesthetic (pos_ = either pos1 or pos2) - assert_eq!(Geom::histogram().aesthetics().required(), &["pos_"]); - assert_eq!(Geom::density().aesthetics().required(), &["pos_"]); + // Statistical geoms require a positional aesthetic (bidirectional: accepts pos1 or pos2) + assert_eq!(Geom::histogram().aesthetics().required(), &["pos1"]); + assert_eq!(Geom::density().aesthetics().required(), &["pos1"]); // Ribbon requires pos2min/pos2max assert_eq!( diff --git a/src/reader/data.rs b/src/reader/data.rs index fac5639a..cb53df53 100644 --- a/src/reader/data.rs +++ b/src/reader/data.rs @@ -368,6 +368,131 @@ mod duckdb_tests { assert!(dataframe.column("__ggsql_aes_pos1__").is_ok()); assert!(dataframe.column("__ggsql_aes_pos2__").is_ok()); } + + #[test] + fn test_ribbon_transposed_orientation() { + use crate::naming; + use crate::plot::layer::orientation::Orientation; + + let reader = + crate::reader::DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + + // Ribbon with y as domain axis and xmin/xmax as value range (transposed) + let query = + "VISUALISE FROM ggsql:airquality DRAW ribbon MAPPING Day AS y, Temp AS xmax, 0.0 AS xmin"; + let result = crate::execute::prepare_data_with_reader(query, &reader); + + // Debug: print the error if any + if let Err(ref e) = result { + eprintln!("Error: {:?}", e); + } + + let result = result.unwrap(); + + // Debug: print orientation and scales + let layer = &result.specs[0].layers[0]; + eprintln!("Layer orientation: {:?}", layer.orientation); + eprintln!( + "Scales: {:?}", + result.specs[0] + .scales + .iter() + .map(|s| (&s.aesthetic, &s.scale_type)) + .collect::>() + ); + eprintln!( + "Layer mappings: {:?}", + layer.mappings.aesthetics.keys().collect::>() + ); + + // Check orientation was detected correctly + assert_eq!( + layer.orientation, + Some(Orientation::Transposed), + "Should detect Transposed orientation" + ); + + let dataframe = result.data.get(&naming::layer_key(0)).unwrap(); + + // The flip-back restores user's original axis assignment: + // After flip-back: + // - pos2 = y (user's domain axis = Date/Day) + // - pos1min = xmin (user's value range min = 0.0) + // - pos1max = xmax (user's value range max = Temp) + // The orientation flag tells the writer how to map to x/y. + let cols: Vec<_> = dataframe.get_column_names().into_iter().collect(); + eprintln!("Columns: {:?}", cols); + + assert!( + dataframe.column("__ggsql_aes_pos2__").is_ok(), + "Should have pos2 (domain axis), got columns: {:?}", + cols + ); + assert!( + dataframe.column("__ggsql_aes_pos1min__").is_ok(), + "Should have pos1min (value range min), got columns: {:?}", + cols + ); + assert!( + dataframe.column("__ggsql_aes_pos1max__").is_ok(), + "Should have pos1max (value range max), got columns: {:?}", + cols + ); + } + + #[test] + fn test_ribbon_transposed_vegalite_encoding() { + use crate::reader::Reader; + use crate::writer::{VegaLiteWriter, Writer}; + + let reader = + crate::reader::DuckDBReader::from_connection_string("duckdb://memory").unwrap(); + + // Ribbon with y as domain axis and xmin/xmax as value range (transposed) + let query = + "VISUALISE FROM ggsql:airquality DRAW ribbon MAPPING Day AS y, Temp AS xmax, 0.0 AS xmin"; + let spec = reader.execute(query).unwrap(); + + let writer = VegaLiteWriter::new(); + let json_str = writer.render(&spec).unwrap(); + let vl_spec: serde_json::Value = serde_json::from_str(&json_str).unwrap(); + + // For transposed ribbon, the encoding should have: + // - y: domain axis (Day) + // - x: value range max (Temp via xmax) + // - x2: value range min (0.0 via xmin) + // The encoding is inside layer[0] since VegaLite uses layered structure + let encoding = &vl_spec["layer"][0]["encoding"]; + assert!( + encoding.get("y").is_some(), + "Should have y encoding for domain axis" + ); + assert!( + encoding.get("x").is_some(), + "Should have x encoding for value max" + ); + assert!( + encoding.get("x2").is_some(), + "Should have x2 encoding for value min" + ); + // Should NOT have ymax/ymin/xmax/xmin (these should be remapped to x/x2/y/y2) + assert!( + encoding.get("ymax").is_none(), + "Should not have ymax encoding" + ); + assert!( + encoding.get("ymin").is_none(), + "Should not have ymin encoding" + ); + assert!( + encoding.get("xmax").is_none(), + "Should not have xmax encoding" + ); + assert!( + encoding.get("xmin").is_none(), + "Should not have xmin encoding" + ); + } } #[cfg(feature = "builtin-data")] diff --git a/src/writer/vegalite/layer.rs b/src/writer/vegalite/layer.rs index b69be4b7..8dcbdab6 100644 --- a/src/writer/vegalite/layer.rs +++ b/src/writer/vegalite/layer.rs @@ -533,21 +533,30 @@ impl GeomRenderer for RibbonRenderer { fn modify_encoding( &self, encoding: &mut Map, - _layer: &Layer, + layer: &Layer, _context: &RenderContext, ) -> Result<()> { - if let Some(ymax) = encoding.remove("ymax") { - encoding.insert("y".to_string(), ymax); + use crate::plot::layer::Orientation; + + let is_horizontal = layer.orientation == Some(Orientation::Transposed); + + // Remap min/max to primary/secondary based on orientation: + // - Aligned (vertical): ymax→y, ymin→y2 + // - Transposed (horizontal): xmax→x, xmin→x2 + let (max_key, min_key, target, target2) = if is_horizontal { + ("xmax", "xmin", "x", "x2") + } else { + ("ymax", "ymin", "y", "y2") + }; + + if let Some(max_val) = encoding.remove(max_key) { + encoding.insert(target.to_string(), max_val); } - if let Some(ymin) = encoding.remove("ymin") { - encoding.insert("y2".to_string(), ymin); + if let Some(min_val) = encoding.remove(min_key) { + encoding.insert(target2.to_string(), min_val); } - // Use row index field to preserve natural data order - // (we've already ordered in SQL via apply_layer_transforms) - encoding.insert( - "order".to_string(), - json!({"field": ROW_INDEX_COLUMN, "type": "quantitative"}), - ); + + // Note: Don't add order encoding for area marks - it interferes with rendering Ok(()) } } From fe1d14b9c13999b3cb0a2b7a90bc5d0c9ea5b18c Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Tue, 10 Mar 2026 14:58:00 +0100 Subject: [PATCH 5/5] preemptively move docs to match main --- doc/syntax/layer/{ => type}/area.qmd | 0 doc/syntax/layer/{ => type}/bar.qmd | 0 doc/syntax/layer/{ => type}/boxplot.qmd | 0 doc/syntax/layer/{ => type}/density.qmd | 0 doc/syntax/layer/{ => type}/errorbar.qmd | 0 doc/syntax/layer/{ => type}/histogram.qmd | 0 doc/syntax/layer/{ => type}/line.qmd | 0 doc/syntax/layer/{ => type}/linear.qmd | 0 doc/syntax/layer/{ => type}/path.qmd | 0 doc/syntax/layer/{ => type}/point.qmd | 0 doc/syntax/layer/{ => type}/polygon.qmd | 0 doc/syntax/layer/{ => type}/ribbon.qmd | 0 doc/syntax/layer/{ => type}/rule.qmd | 0 doc/syntax/layer/{ => type}/segment.qmd | 0 doc/syntax/layer/{ => type}/violin.qmd | 0 15 files changed, 0 insertions(+), 0 deletions(-) rename doc/syntax/layer/{ => type}/area.qmd (100%) rename doc/syntax/layer/{ => type}/bar.qmd (100%) rename doc/syntax/layer/{ => type}/boxplot.qmd (100%) rename doc/syntax/layer/{ => type}/density.qmd (100%) rename doc/syntax/layer/{ => type}/errorbar.qmd (100%) rename doc/syntax/layer/{ => type}/histogram.qmd (100%) rename doc/syntax/layer/{ => type}/line.qmd (100%) rename doc/syntax/layer/{ => type}/linear.qmd (100%) rename doc/syntax/layer/{ => type}/path.qmd (100%) rename doc/syntax/layer/{ => type}/point.qmd (100%) rename doc/syntax/layer/{ => type}/polygon.qmd (100%) rename doc/syntax/layer/{ => type}/ribbon.qmd (100%) rename doc/syntax/layer/{ => type}/rule.qmd (100%) rename doc/syntax/layer/{ => type}/segment.qmd (100%) rename doc/syntax/layer/{ => type}/violin.qmd (100%) diff --git a/doc/syntax/layer/area.qmd b/doc/syntax/layer/type/area.qmd similarity index 100% rename from doc/syntax/layer/area.qmd rename to doc/syntax/layer/type/area.qmd diff --git a/doc/syntax/layer/bar.qmd b/doc/syntax/layer/type/bar.qmd similarity index 100% rename from doc/syntax/layer/bar.qmd rename to doc/syntax/layer/type/bar.qmd diff --git a/doc/syntax/layer/boxplot.qmd b/doc/syntax/layer/type/boxplot.qmd similarity index 100% rename from doc/syntax/layer/boxplot.qmd rename to doc/syntax/layer/type/boxplot.qmd diff --git a/doc/syntax/layer/density.qmd b/doc/syntax/layer/type/density.qmd similarity index 100% rename from doc/syntax/layer/density.qmd rename to doc/syntax/layer/type/density.qmd diff --git a/doc/syntax/layer/errorbar.qmd b/doc/syntax/layer/type/errorbar.qmd similarity index 100% rename from doc/syntax/layer/errorbar.qmd rename to doc/syntax/layer/type/errorbar.qmd diff --git a/doc/syntax/layer/histogram.qmd b/doc/syntax/layer/type/histogram.qmd similarity index 100% rename from doc/syntax/layer/histogram.qmd rename to doc/syntax/layer/type/histogram.qmd diff --git a/doc/syntax/layer/line.qmd b/doc/syntax/layer/type/line.qmd similarity index 100% rename from doc/syntax/layer/line.qmd rename to doc/syntax/layer/type/line.qmd diff --git a/doc/syntax/layer/linear.qmd b/doc/syntax/layer/type/linear.qmd similarity index 100% rename from doc/syntax/layer/linear.qmd rename to doc/syntax/layer/type/linear.qmd diff --git a/doc/syntax/layer/path.qmd b/doc/syntax/layer/type/path.qmd similarity index 100% rename from doc/syntax/layer/path.qmd rename to doc/syntax/layer/type/path.qmd diff --git a/doc/syntax/layer/point.qmd b/doc/syntax/layer/type/point.qmd similarity index 100% rename from doc/syntax/layer/point.qmd rename to doc/syntax/layer/type/point.qmd diff --git a/doc/syntax/layer/polygon.qmd b/doc/syntax/layer/type/polygon.qmd similarity index 100% rename from doc/syntax/layer/polygon.qmd rename to doc/syntax/layer/type/polygon.qmd diff --git a/doc/syntax/layer/ribbon.qmd b/doc/syntax/layer/type/ribbon.qmd similarity index 100% rename from doc/syntax/layer/ribbon.qmd rename to doc/syntax/layer/type/ribbon.qmd diff --git a/doc/syntax/layer/rule.qmd b/doc/syntax/layer/type/rule.qmd similarity index 100% rename from doc/syntax/layer/rule.qmd rename to doc/syntax/layer/type/rule.qmd diff --git a/doc/syntax/layer/segment.qmd b/doc/syntax/layer/type/segment.qmd similarity index 100% rename from doc/syntax/layer/segment.qmd rename to doc/syntax/layer/type/segment.qmd diff --git a/doc/syntax/layer/violin.qmd b/doc/syntax/layer/type/violin.qmd similarity index 100% rename from doc/syntax/layer/violin.qmd rename to doc/syntax/layer/type/violin.qmd