Skip to content

Commit c1979cc

Browse files
teunbrandclaude
andauthored
Defaults for aesthetics (#142)
* Centralize aesthetic metadata in unified DefaultAesthetics structure Restructure GeomAesthetics to prepare for centralized default aesthetic values. Each geom now defines all aesthetic metadata (required, optional, delayed) in a single defaults field, enabling future control over visual appearance defaults. Key changes: - Rename GeomAesthetics to DefaultAesthetics with unified defaults field - Add Required, Null, Delayed variants to DefaultAestheticValue enum - Derive supported/required lists via helper methods instead of separate fields - Update all 22 geom implementations with new structure - Fix semantic bugs where MAPPING validation incorrectly included Delayed aesthetics Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Apply default aesthetic values from geom definitions in writer Implement Step 4 of the plan: writer now applies default aesthetic values from each geom's DefaultAesthetics when building encodings. Defaults are applied with lowest priority (after MAPPING and SETTING). Key changes: - Add logic in build_layer_encoding() to iterate over geom defaults - Skip if encoding already set by MAPPING or SETTING - Convert DefaultAestheticValue to AestheticValue via to_aesthetic_value() - Reuse existing build_encoding_channel() for unit conversions - Skip Required, Null, and Delayed variants (no literal defaults to apply) Precedence order: MAPPING > SETTING > defaults (geom) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Remove hardcoded aesthetic defaults from all renderers Remove redundant hardcoded defaults from BoxplotRenderer and PolygonRenderer. These defaults are now handled by the centralized default aesthetics system in build_layer_encoding(). Changes: - BoxplotRenderer: removed default_stroke, default_fill, default_linewidth - PolygonRenderer: removed hardcoded fill and stroke ("#888888") Encoding-level properties (from geom defaults) override mark-level properties, so the hardcoded values were redundant. All defaults now come from each geom's DefaultAesthetics definition. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add comprehensive tests for default aesthetic system Add unit and integration tests to verify default aesthetic behavior: Unit tests (types.rs): - test_default_aesthetics_methods: Comprehensive test of all helper methods (get, names, supported, required, is_supported, contains, is_required) on a DefaultAesthetics instance with various value types Integration tests (vegalite/mod.rs): - test_default_aesthetics_applied: Verify defaults appear in Vega-Lite output (stroke="black", opacity=1.0) - test_setting_overrides_default: Verify SETTING takes precedence over defaults - test_mapping_overrides_default: Verify MAPPING takes precedence over defaults - test_null_defaults_not_applied: Verify Null variants don't create encodings Also adds get() helper method to DefaultAesthetics for cleaner test code. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Translate linetype strings to strokeDash arrays in writer. Use existing linetype_to_stroke_dash() function to convert linetype strings to Vega-Lite strokeDash arrays in two places: - SETTING parameters (e.g., SETTING linetype => 'dashed') - Default aesthetic values (e.g., line geom's default "solid") Named patterns like "solid", "dashed", "dotted" are converted to numeric arrays ([6,4] for dashed), while unrecognized patterns pass through as strings. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Adjust some aesthetics to taste * Refactor SETTING and defaults processing in Vega-Lite writer Unify SETTING parameter and default aesthetic processing into a single loop to eliminate code duplication and clarify precedence order. Changes: - Merge separate SETTING and defaults loops into unified loop - Use build_encoding_channel() for both (eliminates conversion duplication) - Make precedence explicit: SETTING > defaults (both checked per aesthetic) - Remove linetype_to_stroke_dash import (now only used in encoding.rs) Benefits: - Single source of truth for SETTING/defaults precedence logic - All conversions (size, linewidth, linetype) go through same code path - Reduced code: ~40 lines → ~28 lines - Prepares for future writers (ggplot2, plotters) to reuse precedence logic Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Centralize SETTING/defaults precedence logic in Layer Add Layer::get_aesthetic_value() to centralize SETTING > defaults precedence logic, preparing for multiple writers (ggplot2, plotters). Changes: - Add DefaultAestheticValue::to_parameter_value() to extract literal values (returns ParameterValue, not Option - uses Null for non-literals) - Refactor to_aesthetic_value() to reuse to_parameter_value() (eliminates duplication) - Add Layer::get_aesthetic_value() for SETTING > defaults resolution - Update Vega-Lite writer to use get_aesthetic_value() (28 lines → 18 lines) Benefits: - Precedence logic centralized in Layer (not duplicated per writer) - Future writers can reuse get_aesthetic_value() for consistent behavior - Conversions (size, linewidth, linetype) remain writer-specific - Cleaner, more maintainable code with simpler signatures Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * remove dangling test & cargo fmt * It turns out the unreachable statement did in fact become reachable * review area/density opacity * Move aesthetic resolution from writer to execution phase Aesthetic harmonization (MAPPING > SETTING > defaults) now happens once during execution rather than per-writer. This creates a single source of truth for aesthetic values. Changes: - Add resolved_aesthetics field to Layer (HashMap of resolved values) - Add Layer::resolve_aesthetics() method called during execution - Remove Layer::get_aesthetic_value() (logic now inlined) - Simplify Vega-Lite writer to use resolved_aesthetics directly - Add comprehensive tests for resolution logic This ensures all writers handle aesthetics consistently and reduces duplication across writer implementations. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Merge resolved_aesthetics into mappings for single source of truth Consolidate aesthetic resolution by removing the separate resolved_aesthetics field and inserting SETTING/defaults directly into mappings as Literal values. The key insight is timing: resolve_aesthetics() now runs AFTER query literals have been converted to columns, ensuring proper distinction. Changes: - Remove resolved_aesthetics field from Layer struct - Update resolve_aesthetics() to insert into mappings as AestheticValue::Literal - Move resolve_aesthetics() call to after remappings (line 733) - Simplify Vega-Lite writer to single loop over mappings - Update all tests to check mappings instead of resolved_aesthetics - Add comprehensive documentation explaining query literal vs SETTING/default distinction Flow: 1. Query literals ('foo' AS color) → parsed as Literal in mappings 2. build_layer_select_list() → converts to SQL columns 3. update_mappings_for_aesthetic_columns() → Literal → Column in mappings 4. resolve_aesthetics() → adds SETTING/defaults as NEW Literals (key change) 5. Writer → Column gets scales, Literal renders as constant value This ensures: - Query literals can have scales applied (they're columns) - SETTING/defaults remain constant values (they're Literals) - Single source of truth for all aesthetic values - Correct precedence: MAPPING > REMAPPING > SETTING > defaults Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix broken test expectation * cargo fmt * set opaque default fill for area/density * retarget opacity to fillOpacity when fill is supported * Don't bake in opacity into the fill colour * update expectations based on new defaults --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 850a362 commit c1979cc

33 files changed

Lines changed: 899 additions & 355 deletions

doc/syntax/layer/density.qmd

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,14 @@ A typical KDE computation with different groups:
7373

7474
```{ggsql}
7575
VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins
76-
DRAW density SETTING opacity => 0.8
76+
DRAW density
7777
```
7878

7979
Changing the relative bandwidth through the `adjust` setting.
8080

8181
```{ggsql}
8282
VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins
83-
DRAW density SETTING opacity => 0.8, adjust => 0.1
83+
DRAW density SETTING adjust => 0.1
8484
```
8585

8686
Stacking the different groups instead of overlaying them.
@@ -94,9 +94,7 @@ Using weighted estimates by mapping a column to the optional weight aesthetic. N
9494

9595
```{ggsql}
9696
VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins
97-
DRAW density
98-
MAPPING body_mass AS weight
99-
SETTING opacity => 0.8
97+
DRAW density MAPPING body_mass AS weight
10098
```
10199

102100
If you want to compare a histogram and a density layer, you can use the `intensity` computed variable to match the histogram scale.
@@ -114,8 +112,6 @@ Note the relative height of the groups.
114112

115113
```{ggsql}
116114
VISUALISE bill_dep AS x, species AS colour FROM ggsql:penguins
117-
DRAW density
118-
REMAPPING intensity AS y
119-
SETTING opacity => 0.8
115+
DRAW density REMAPPING intensity AS y
120116
```
121117

doc/syntax/layer/point.qmd

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ Create a classic scatterplot
3535
VISUALISE FROM ggsql:penguins
3636
DRAW point
3737
MAPPING bill_len AS x, bill_dep AS y, species AS fill
38-
SETTING size => 30
3938
```
4039

4140
Map to size to create a bubble chart
@@ -52,6 +51,5 @@ Use filter to only plot a subset of the data
5251
VISUALISE FROM ggsql:penguins
5352
DRAW point
5453
MAPPING bill_len AS x, bill_dep AS y, species AS fill
55-
SETTING size => 30
5654
FILTER sex = 'female'
5755
```

src/execute/mod.rs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ use crate::reader::DuckDBReader;
5050
fn validate(layers: &[Layer], layer_schemas: &[Schema]) -> Result<()> {
5151
for (idx, (layer, schema)) in layers.iter().zip(layer_schemas.iter()).enumerate() {
5252
let schema_columns: HashSet<&str> = schema.iter().map(|c| c.name.as_str()).collect();
53-
let supported = layer.geom.aesthetics().supported;
53+
let supported = layer.geom.aesthetics().supported();
5454

5555
// Validate required aesthetics for this geom
5656
layer
@@ -97,14 +97,10 @@ fn validate(layers: &[Layer], layer_schemas: &[Schema]) -> Result<()> {
9797
}
9898

9999
// Validate remapping target aesthetics are supported by geom
100-
// Target can be in supported OR hidden (hidden = valid REMAPPING targets but not MAPPING targets)
100+
// REMAPPING can target any aesthetic (including Delayed ones from stat transforms)
101101
let aesthetics_info = layer.geom.aesthetics();
102102
for target_aesthetic in layer.remappings.aesthetics.keys() {
103-
let is_supported = aesthetics_info
104-
.supported
105-
.contains(&target_aesthetic.as_str());
106-
let is_hidden = aesthetics_info.hidden.contains(&target_aesthetic.as_str());
107-
if !is_supported && !is_hidden {
103+
if !aesthetics_info.contains(target_aesthetic) {
108104
return Err(GgsqlError::ValidationError(format!(
109105
"Layer {}: REMAPPING targets unsupported aesthetic '{}' for geom '{}'",
110106
idx + 1,
@@ -157,7 +153,7 @@ fn validate(layers: &[Layer], layer_schemas: &[Schema]) -> Result<()> {
157153
fn merge_global_mappings_into_layers(specs: &mut [Plot], layer_schemas: &[Schema]) {
158154
for spec in specs {
159155
for (layer, schema) in spec.layers.iter_mut().zip(layer_schemas.iter()) {
160-
let supported = layer.geom.aesthetics().supported;
156+
let supported = layer.geom.aesthetics().supported();
161157
let schema_columns: HashSet<&str> = schema.iter().map(|c| c.name.as_str()).collect();
162158

163159
// 1. First merge explicit global aesthetics (layer overrides global)
@@ -180,14 +176,14 @@ fn merge_global_mappings_into_layers(specs: &mut [Plot], layer_schemas: &[Schema
180176
// 2. Smart wildcard expansion: only expand to columns that exist in schema
181177
let has_wildcard = layer.mappings.wildcard || spec.global_mappings.wildcard;
182178
if has_wildcard {
183-
for &aes in supported {
179+
for aes in &supported {
184180
// Only create mapping if column exists in the schema
185-
if schema_columns.contains(aes) {
181+
if schema_columns.contains(*aes) {
186182
layer
187183
.mappings
188184
.aesthetics
189185
.entry(crate::parser::builder::normalise_aes_name(aes))
190-
.or_insert(AestheticValue::standard_column(aes));
186+
.or_insert(AestheticValue::standard_column(*aes));
191187
}
192188
}
193189
}
@@ -228,10 +224,10 @@ fn split_color_aesthetic(spec: &mut Plot) {
228224
// 2. Split color mapping to fill/stroke in layers, then remove color
229225
for layer in &mut spec.layers {
230226
if let Some(color_value) = layer.mappings.aesthetics.get("color").cloned() {
231-
let supported = layer.geom.aesthetics().supported;
227+
let aesthetics = layer.geom.aesthetics();
232228

233229
for &aes in &["stroke", "fill"] {
234-
if supported.contains(&aes) {
230+
if aesthetics.is_supported(aes) {
235231
layer
236232
.mappings
237233
.aesthetics
@@ -248,10 +244,10 @@ fn split_color_aesthetic(spec: &mut Plot) {
248244
// 3. Split color parameter (SETTING) to fill/stroke in layers
249245
for layer in &mut spec.layers {
250246
if let Some(color_value) = layer.parameters.get("color").cloned() {
251-
let supported = layer.geom.aesthetics().supported;
247+
let aesthetics = layer.geom.aesthetics();
252248

253249
for &aes in &["stroke", "fill"] {
254-
if supported.contains(&aes) {
250+
if aesthetics.is_supported(aes) {
255251
layer
256252
.parameters
257253
.entry(aes.to_string())
@@ -1127,6 +1123,11 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
11271123
// Update layer mappings for all layers (even if data shared)
11281124
l.update_mappings_for_remappings();
11291125
}
1126+
1127+
// Resolve aesthetics (SETTING/defaults) after all mapping updates
1128+
// This ensures query literals have been converted to columns, and SETTING/defaults
1129+
// are added as new Literal entries that remain as constant values
1130+
l.resolve_aesthetics();
11301131
}
11311132

11321133
// Validate we have some data (every layer should have its own data)

src/plot/layer/geom/abline.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! AbLine geom implementation
22
3-
use super::{GeomAesthetics, GeomTrait, GeomType};
3+
use super::{DefaultAesthetics, GeomTrait, GeomType};
4+
use crate::plot::types::DefaultAestheticValue;
45

56
/// AbLine geom - lines with slope and intercept
67
#[derive(Debug, Clone, Copy)]
@@ -11,18 +12,16 @@ impl GeomTrait for AbLine {
1112
GeomType::AbLine
1213
}
1314

14-
fn aesthetics(&self) -> GeomAesthetics {
15-
GeomAesthetics {
16-
supported: &[
17-
"slope",
18-
"intercept",
19-
"stroke",
20-
"linetype",
21-
"linewidth",
22-
"opacity",
15+
fn aesthetics(&self) -> DefaultAesthetics {
16+
DefaultAesthetics {
17+
defaults: &[
18+
("slope", DefaultAestheticValue::Required),
19+
("intercept", DefaultAestheticValue::Required),
20+
("stroke", DefaultAestheticValue::String("black")),
21+
("linewidth", DefaultAestheticValue::Number(1.0)),
22+
("opacity", DefaultAestheticValue::Number(1.0)),
23+
("linetype", DefaultAestheticValue::String("solid")),
2324
],
24-
required: &["slope", "intercept"],
25-
hidden: &[],
2625
}
2726
}
2827
}

src/plot/layer/geom/area.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
//! Area geom implementation
22
3-
use crate::plot::{DefaultParam, DefaultParamValue};
3+
use crate::plot::{types::DefaultAestheticValue, DefaultParam, DefaultParamValue};
44

5-
use super::{GeomAesthetics, GeomTrait, GeomType};
5+
use super::{DefaultAesthetics, GeomTrait, GeomType};
66

77
/// Area geom - filled area charts
88
#[derive(Debug, Clone, Copy)]
@@ -13,19 +13,17 @@ impl GeomTrait for Area {
1313
GeomType::Area
1414
}
1515

16-
fn aesthetics(&self) -> GeomAesthetics {
17-
GeomAesthetics {
18-
supported: &[
19-
"x",
20-
"y",
21-
"fill",
22-
"stroke",
23-
"opacity",
24-
"linewidth",
25-
// "linetype", // vegalite doesn't support strokeDash
16+
fn aesthetics(&self) -> DefaultAesthetics {
17+
DefaultAesthetics {
18+
defaults: &[
19+
("x", DefaultAestheticValue::Required),
20+
("y", DefaultAestheticValue::Required),
21+
("fill", DefaultAestheticValue::String("black")),
22+
("stroke", DefaultAestheticValue::String("black")),
23+
("opacity", DefaultAestheticValue::Number(0.8)),
24+
("linewidth", DefaultAestheticValue::Number(1.0)),
25+
("linetype", DefaultAestheticValue::String("solid")),
2626
],
27-
required: &["x", "y"],
28-
hidden: &[],
2927
}
3028
}
3129

src/plot/layer/geom/arrow.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Arrow geom implementation
22
3-
use super::{GeomAesthetics, GeomTrait, GeomType};
3+
use super::{DefaultAesthetics, GeomTrait, GeomType};
4+
use crate::plot::types::DefaultAestheticValue;
45

56
/// Arrow geom - line segments with arrowheads
67
#[derive(Debug, Clone, Copy)]
@@ -11,20 +12,19 @@ impl GeomTrait for Arrow {
1112
GeomType::Arrow
1213
}
1314

14-
fn aesthetics(&self) -> GeomAesthetics {
15-
GeomAesthetics {
16-
supported: &[
17-
"x",
18-
"y",
19-
"xend",
20-
"yend",
21-
"stroke",
22-
"linetype",
23-
"linewidth",
24-
"opacity",
15+
fn aesthetics(&self) -> DefaultAesthetics {
16+
DefaultAesthetics {
17+
defaults: &[
18+
("x", DefaultAestheticValue::Required),
19+
("y", DefaultAestheticValue::Required),
20+
("xend", DefaultAestheticValue::Required),
21+
("yend", DefaultAestheticValue::Required),
22+
("stroke", DefaultAestheticValue::String("black")),
23+
("linewidth", DefaultAestheticValue::Number(1.0)),
24+
("opacity", DefaultAestheticValue::Number(1.0)),
25+
("linetype", DefaultAestheticValue::String("solid")),
26+
("fill", DefaultAestheticValue::Null),
2527
],
26-
required: &["x", "y", "xend", "yend"],
27-
hidden: &[],
2828
}
2929
}
3030
}

src/plot/layer/geom/bar.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::collections::HashMap;
44
use std::collections::HashSet;
55

66
use super::types::get_column_name;
7-
use super::{DefaultParam, DefaultParamValue, GeomAesthetics, GeomTrait, GeomType, StatResult};
7+
use super::{DefaultAesthetics, DefaultParam, DefaultParamValue, GeomTrait, GeomType, StatResult};
88
use crate::naming;
99
use crate::plot::types::{DefaultAestheticValue, ParameterValue};
1010
use crate::{DataFrame, GgsqlError, Mappings, Result};
@@ -20,15 +20,23 @@ impl GeomTrait for Bar {
2020
GeomType::Bar
2121
}
2222

23-
fn aesthetics(&self) -> GeomAesthetics {
24-
GeomAesthetics {
23+
fn aesthetics(&self) -> DefaultAesthetics {
24+
DefaultAesthetics {
2525
// Bar supports optional x and y - stat decides aggregation
2626
// If x is missing: single bar showing total
2727
// If y is missing: stat computes COUNT or SUM(weight)
2828
// weight: optional, if mapped uses SUM(weight) instead of COUNT(*)
29-
supported: &["x", "y", "weight", "fill", "stroke", "width", "opacity"],
30-
required: &[],
31-
hidden: &[],
29+
// width is a parameter, not an aesthetic.
30+
// if we ever want to make 'width' an aesthetic, we'd probably need to
31+
// translate it to 'size'.
32+
defaults: &[
33+
("x", DefaultAestheticValue::Null), // Optional - stat may provide
34+
("y", DefaultAestheticValue::Null), // Optional - stat may compute
35+
("weight", DefaultAestheticValue::Null),
36+
("fill", DefaultAestheticValue::String("black")),
37+
("stroke", DefaultAestheticValue::String("black")),
38+
("opacity", DefaultAestheticValue::Number(0.8)),
39+
],
3240
}
3341
}
3442

src/plot/layer/geom/boxplot.rs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use std::collections::HashMap;
44

5-
use super::{GeomAesthetics, GeomTrait, GeomType};
5+
use super::{DefaultAesthetics, GeomTrait, GeomType};
66
use crate::{
77
naming,
88
plot::{
@@ -21,22 +21,22 @@ impl GeomTrait for Boxplot {
2121
GeomType::Boxplot
2222
}
2323

24-
fn aesthetics(&self) -> GeomAesthetics {
25-
GeomAesthetics {
26-
supported: &[
27-
"x",
28-
"y",
29-
"fill",
30-
"stroke",
31-
"opacity",
32-
"linetype",
33-
"linewidth",
34-
"size",
35-
"shape",
24+
fn aesthetics(&self) -> DefaultAesthetics {
25+
DefaultAesthetics {
26+
defaults: &[
27+
("x", DefaultAestheticValue::Required),
28+
("y", DefaultAestheticValue::Required),
29+
("stroke", DefaultAestheticValue::String("black")),
30+
("fill", DefaultAestheticValue::String("white")),
31+
("linewidth", DefaultAestheticValue::Number(1.0)),
32+
("opacity", DefaultAestheticValue::Number(0.8)),
33+
("linetype", DefaultAestheticValue::String("solid")),
34+
("size", DefaultAestheticValue::Number(3.0)),
35+
("shape", DefaultAestheticValue::String("circle")),
36+
// Internal aesthetics produced by stat transform
37+
("type", DefaultAestheticValue::Delayed),
38+
("yend", DefaultAestheticValue::Delayed),
3639
],
37-
required: &["x", "y"],
38-
// Internal aesthetics produced by stat transform
39-
hidden: &["type", "y", "yend"],
4040
}
4141
}
4242

@@ -547,21 +547,21 @@ mod tests {
547547
let boxplot = Boxplot;
548548
let aes = boxplot.aesthetics();
549549

550-
assert!(aes.required.contains(&"x"));
551-
assert!(aes.required.contains(&"y"));
552-
assert_eq!(aes.required.len(), 2);
550+
assert!(aes.is_required("x"));
551+
assert!(aes.is_required("y"));
552+
assert_eq!(aes.required().len(), 2);
553553
}
554554

555555
#[test]
556556
fn test_boxplot_aesthetics_supported() {
557557
let boxplot = Boxplot;
558558
let aes = boxplot.aesthetics();
559559

560-
assert!(aes.supported.contains(&"x"));
561-
assert!(aes.supported.contains(&"y"));
562-
assert!(aes.supported.contains(&"fill"));
563-
assert!(aes.supported.contains(&"stroke"));
564-
assert!(aes.supported.contains(&"opacity"));
560+
assert!(aes.is_supported("x"));
561+
assert!(aes.is_supported("y"));
562+
assert!(aes.is_supported("fill"));
563+
assert!(aes.is_supported("stroke"));
564+
assert!(aes.is_supported("opacity"));
565565
}
566566

567567
#[test]

0 commit comments

Comments
 (0)