Open
Conversation
Introduces PLACE as a new top-level clause for creating annotation layers with literal values, similar to ggplot2's annotate() function. Changes: - Grammar: Add place_clause accepting geom and optional SETTING - DataSource: Add Annotation variant to mark annotation layers - Builder: Handle place_clause and set DataSource::Annotation - Execution: Add stub for annotation data source generation - Tests: Add parser test for PLACE clause PLACE layers use DataSource::Annotation as a marker and don't inherit global mappings. All aesthetic values come from SETTING parameters. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Completes the execution pipeline for PLACE layers: Changes: - Skip annotation layers in merge_global_mappings_into_layers() to prevent inheritance of global mappings from VISUALISE - Clarify annotation data source comment: 1-row dummy satisfies execution pipeline requirements (schema detection, type resolution) even though SETTING literals render as Vega-Lite datum values Annotation layers now properly isolated from global mappings while still working with the existing execution infrastructure. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enables annotation layers to participate in scale training and coordinate transformation pipeline by moving positional/required aesthetics from SETTING parameters to mappings. Changes: - Transform parameter keys to internal space (x → pos1, y → pos2) - Move positional and required aesthetics from parameters to mappings for annotation layers in transform_aesthetics_to_internal() - Refactor place_clause handling in builder.rs into build_place_layer() - Add comprehensive test coverage: parser, execution, validation, and rendering tests verifying field vs value encoding Annotation position aesthetics now materialize as columns, enabling them to affect scale ranges and be properly encoded in Vega-Lite output as field encodings (not datum values). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Change DataSource::Annotation from a unit variant to a tuple variant containing the number of rows to generate (usize). This prepares for array expansion support in PLACE annotation layers. Changes: - DataSource::Annotation becomes DataSource::Annotation(usize) - Initialize annotation layers with length 1 in build_place_layer() - Generate multi-row VALUES clause in determine_layer_source() when n > 1 - Update all matches!() checks to use DataSource::Annotation(_) - Update test assertions to use pattern matching The length will be updated in process_annotation_layers() when arrays are present (to be implemented next). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit implements array parameter recycling for annotation layers, allowing scalars and length-1 arrays to be automatically recycled to match the length of the longest array parameter. Changes: - Updated DataSource::Annotation to store row count: Annotation(usize) - Added recycle_value_to_length() helper in plot/main.rs - Implemented process_annotation_layers() to handle array recycling - Updated schema inference to handle arrays in literal aesthetic mappings - Modified literal_to_sql() to generate CASE WHEN statements for arrays - Fixed SQL generation for multi-row VALUES clause in annotation layers - Added tests for array recycling and mismatched length validation Recycling Rules: - Scalars recycle to max array length - Length-1 arrays recycle to max array length - All non-1 arrays must have same length (error on mismatch) - Only required/positional aesthetics get recycled when moved to mappings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit fixes a bug where PLACE text annotation layers with array parameters (e.g., label => [1, 'foo']) displayed incorrectly. Changes: - Add ArrayElement::homogenize() to coerce mixed-type arrays to a common type - Process annotation layers to move array parameters to mappings - Skip non-positional aesthetics for annotation layers in scale training - Remove string-to-number parsing in JSON serialization to preserve types The fix ensures that arrays like [1, 'foo'] are homogenized to ["1", "foo"] and rendered correctly as separate labels in the visualization. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extract standalone functions into methods on their respective types for better encapsulation and API design: - Add ArrayElement::to_sql() for SQL literal conversion - Add ParameterValue::to_sql() for SQL literal conversion (including CASE statements for arrays) - Add ParameterValue::rep(n) for array recycling (replaces recycle_value_to_length) - Add ParameterValue::to_array_element() helper for scalar conversion Key improvements: - Simplified rep() to only homogenize arrays when arr.len() == n - Unified scalar handling through to_array_element() catch-all - Removed literal_to_sql() standalone function - Removed recycle_value_to_length() standalone function Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes issue where PLACE layer literals (e.g., stroke => ['red', 'blue']) would incorrectly use scales from other layers. Changes: - Add is_scaled field to AestheticValue::Column - Add identity_column() helper for creating unscaled columns - Mark annotation layer non-positional literals with is_scaled: false - Use identity_scale when is_scaled is false in encoding Example: PLACE text with stroke => ['red', 'blue'] now renders with literal colors instead of being transformed by scales from other layers. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Simplifies annotation layer handling by consolidating SQL generation logic: - Unified layer_source_query() to handle all source types including annotations - Moved determine_layer_source() from casting.rs to layer.rs and merged into layer_source_query() - Removed CASE statement approach in favor of direct VALUES clause generation - build_annotation_values_clause() now generates single SELECT...FROM VALUES statement - Updated ParameterValue::to_sql() to panic on arrays (arrays handled separately) - Cleaned up unused imports in casting.rs This eliminates redundant function calls and awkward unreachable!() cases, making the annotation SQL generation more straightforward. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Improves separation of concerns by keeping AST stateless: - Changed DataSource::Annotation from Annotation(usize) to Annotation (unit variant) - AST no longer carries execution state (row count) - Moved array recycling logic from process_annotation_layers() to build_annotation_values_clause() - Recycling now happens on-the-fly during SQL generation - Validates array lengths and returns error on mismatch - Simplified process_annotation_layers() from ~85 to ~40 lines The AST now purely describes what the user wrote, while execution handles data preparation and recycling. All 16 annotation tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implements a cleaner approach for handling annotation aesthetics: - Added AnnotationColumn variant to AestheticValue enum - AnnotationColumn used only for non-positional annotation aesthetics (color, size, etc) - Positional annotation aesthetics (x, y) use regular Column variant - Removed is_scaled field from Column variant - Scale training automatically skips AnnotationColumn variants - Vega-Lite writer uses identity scale for AnnotationColumn This makes the distinction explicit in the type system rather than carrying state in a boolean flag. Positional annotations participate in scale training (data coordinate space) while non-positional annotations use identity scales (visual space). All 16 annotation tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
NULL aesthetics in PLACE SETTING mean "use geom default", not "create a mapping". Filter them out in process_annotation_layers() to prevent them from reaching VALUES clause generation or literal_to_series(). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Reorganize test suite to replace integration tests with unit tests where appropriate, improving test speed and clarity. Added unit tests: - src/execute/layer.rs: 5 tests for build_annotation_values_clause() * Single scalar values * Array recycling * Mismatched array lengths (error case) * Multiple arrays with same length * Mixed types (number, string, boolean) - src/plot/main.rs: 6 tests for process_annotation_layers() * Moves positional aesthetics to mappings * Moves required aesthetics to mappings * Moves array parameters to mappings * Filters NULL aesthetics * Skips non-annotation layers * Preserves existing mappings Removed redundant integration tests: - test_place_array_recycling_scalar (now unit test) - test_place_array_mismatched_lengths_error (now unit test) All 12 annotation-related tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Annotation layers now support stat geoms (histogram, density, etc.) by using the same column→aesthetic renaming pipeline as regular layers. Previously, annotation layers created prefixed aesthetic columns directly (__ggsql_aes_pos1__), causing naming conflicts when stat transforms tried to rename their output columns to the same names. Changes: - Use raw aesthetic names in VALUES clause (pos1, not __ggsql_aes_pos1__) - Move annotation processing from parse-time to execution-time - Rename build_annotation_values_clause() to process_annotation_layer() - Fix is_scaled parameter (was inverted for Column vs AnnotationColumn) - Make label required for text geom - Flatten nested conditionals with early exits - Remove obsolete Plot::process_annotation_layers() Positional aesthetics now correctly participate in scale training with data-derived domains, while non-positional aesthetics use identity scales. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR aims to fix #115.
It introduces a variant for
DRAWcalledPLACE(name is up for debate), which only supports theSETTINGclause.After building the AST, these are just regular layer with
DataSource::Annotation(n).Here is how it treats variables.
SETTINGin DRAW clauses.VISUALISE-> IgnoredOnce text/rectangle PRs are in, we can use this in examples for these layers