Skip to content

Commit 583a420

Browse files
thomasp85thomasp85
committed
streamline orientation checks
1 parent fd02022 commit 583a420

5 files changed

Lines changed: 30 additions & 79 deletions

File tree

‎src/execute/layer.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! transformations, stat transforms, and post-query operations.
55
66
use crate::plot::aesthetic::{is_positional_aesthetic, AestheticContext};
7+
use crate::plot::layer::is_transposed;
78
use crate::plot::{
89
AestheticValue, DefaultAestheticValue, Layer, ParameterValue, Scale, Schema, SqlTypeNames,
910
StatResult,
@@ -361,12 +362,7 @@ where
361362

362363
// Orientation detection and initial flip was already done in mod.rs before
363364
// build_layer_base_query. We just check if we need to flip back after stat.
364-
let needs_flip = layer
365-
.parameters
366-
.get("orientation")
367-
.and_then(|v| v.as_str())
368-
.map(|s| s == "transposed")
369-
.unwrap_or(false);
365+
let needs_flip = is_transposed(layer, scales);
370366

371367
// Build the aesthetic-named schema for stat transforms
372368
// Note: Mappings were already flipped in mod.rs if needed, so schema reflects normalized orientation

‎src/execute/mod.rs‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use crate::naming;
2626
use crate::parser;
2727
use crate::plot::aesthetic::{is_positional_aesthetic, AestheticContext};
2828
use crate::plot::facet::{resolve_properties as resolve_facet_properties, FacetDataContext};
29+
use crate::plot::layer::is_transposed;
2930
use crate::plot::{AestheticValue, Layer, Scale, ScaleTypeKind, Schema};
3031
use crate::{DataFrame, GgsqlError, Plot, Result};
3132
use std::collections::{HashMap, HashSet};
@@ -1031,7 +1032,7 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
10311032
// `column AS __ggsql_aes_pos1__`.
10321033
{
10331034
use crate::plot::layer::orientation::{
1034-
flip_mappings, geom_has_implicit_orientation, resolve_orientation, TRANSPOSED,
1035+
flip_mappings, geom_has_implicit_orientation, resolve_orientation,
10351036
};
10361037
use crate::plot::ParameterValue;
10371038
let scales = specs[0].scales.clone();
@@ -1073,7 +1074,7 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
10731074
"orientation".to_string(),
10741075
ParameterValue::String(orientation.to_string()),
10751076
);
1076-
if orientation == TRANSPOSED {
1077+
if is_transposed(layer, &scales) {
10771078
flip_mappings(layer);
10781079
// Also flip column names in type_info to match the flipped mappings
10791080
if layer_idx < layer_type_info.len() {
@@ -1178,12 +1179,7 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
11781179
for (idx, q) in layer_queries.iter().enumerate() {
11791180
let layer = &mut specs[0].layers[idx];
11801181
let remappings_key = serde_json::to_string(&layer.remappings).unwrap_or_default();
1181-
let needs_flip = layer
1182-
.parameters
1183-
.get("orientation")
1184-
.and_then(|v| v.as_str())
1185-
.map(|s| s == "transposed")
1186-
.unwrap_or(false);
1182+
let needs_flip = is_transposed(layer, &scales);
11871183
let config_key = (q.clone(), remappings_key, needs_flip);
11881184

11891185
if let Some(existing_key) = config_to_key.get(&config_key) {
@@ -1223,13 +1219,7 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
12231219
// with ALIGNED orientation names) but BEFORE update_mappings_for_remappings
12241220
// (which uses remapping keys to create mapping entries).
12251221
// Phase 4.5 will then flip the DataFrame columns to match.
1226-
let needs_flip = l
1227-
.parameters
1228-
.get("orientation")
1229-
.and_then(|v| v.as_str())
1230-
.map(|s| s == "transposed")
1231-
.unwrap_or(false);
1232-
if needs_flip {
1222+
if is_transposed(l, &scales) {
12331223
crate::plot::layer::orientation::flip_remappings(l);
12341224
}
12351225

@@ -1249,18 +1239,13 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
12491239
// All positional columns (stat-produced and literal) are flipped together.
12501240
let mut flipped_keys: HashSet<String> = HashSet::new();
12511241
for layer in specs[0].layers.iter() {
1252-
let needs_flip = layer
1253-
.parameters
1254-
.get("orientation")
1255-
.and_then(|v| v.as_str())
1256-
.map(|s| s == "transposed")
1257-
.unwrap_or(false);
1258-
if needs_flip {
1242+
if is_transposed(layer, &scales) {
12591243
if let Some(ref key) = layer.data_key {
12601244
if flipped_keys.insert(key.clone()) {
12611245
// First time flipping this data key
12621246
if let Some(df) = data_map.remove(key) {
1263-
let flipped_df = layer::flip_dataframe_positional_columns(df, &aesthetic_ctx);
1247+
let flipped_df =
1248+
layer::flip_dataframe_positional_columns(df, &aesthetic_ctx);
12641249
data_map.insert(key.clone(), flipped_df);
12651250
}
12661251
}

‎src/plot/layer/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub mod orientation;
1616
pub mod position;
1717

1818
// Re-export orientation functions and constants
19-
pub use orientation::{is_transposed, ALIGNED, TRANSPOSED};
19+
pub use orientation::is_transposed;
2020

2121
// Re-export geom types for convenience
2222
pub use geom::{

‎src/writer/vegalite/layer.rs‎

Lines changed: 18 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//! sensible defaults for standard behavior.
99
1010
use crate::plot::layer::geom::GeomType;
11+
use crate::plot::layer::is_transposed;
1112
use crate::plot::ParameterValue;
1213
use crate::writer::vegalite::POINTS_TO_PIXELS;
1314
use crate::{naming, AestheticValue, DataFrame, Geom, GgsqlError, Layer, Result};
@@ -243,6 +244,7 @@ pub trait GeomRenderer: Send + Sync {
243244
_layer: &Layer,
244245
_data_key: &str,
245246
_prepared: &PreparedData,
247+
_context: &RenderContext,
246248
) -> Result<Vec<Value>> {
247249
Ok(vec![layer_spec])
248250
}
@@ -269,20 +271,15 @@ impl GeomRenderer for BarRenderer {
269271
&self,
270272
layer_spec: &mut Value,
271273
layer: &Layer,
272-
_context: &RenderContext,
274+
context: &RenderContext,
273275
) -> Result<()> {
274276
let width = match layer.parameters.get("width") {
275277
Some(ParameterValue::Number(w)) => *w,
276278
_ => 0.9,
277279
};
278280

279281
// For horizontal bars, use "height" for band size; for vertical, use "width"
280-
let is_horizontal = layer
281-
.parameters
282-
.get("orientation")
283-
.and_then(|v| v.as_str())
284-
.map(|s| s == "transposed")
285-
.unwrap_or(false);
282+
let is_horizontal = is_transposed(layer, context.scales);
286283

287284
// For dodged bars, use expression-based size with the adjusted width
288285
// For non-dodged bars, use band-relative size
@@ -450,19 +447,14 @@ impl GeomRenderer for LinearRenderer {
450447
&self,
451448
encoding: &mut Map<String, Value>,
452449
layer: &Layer,
453-
_context: &RenderContext,
450+
context: &RenderContext,
454451
) -> Result<()> {
455452
// Remove coefficient and intercept from encoding - they're only used in transforms
456453
encoding.remove("coef");
457454
encoding.remove("intercept");
458455

459456
// Check orientation
460-
let is_horizontal = layer
461-
.parameters
462-
.get("orientation")
463-
.and_then(|v| v.as_str())
464-
.map(|s| s == "transposed")
465-
.unwrap_or(false);
457+
let is_horizontal = is_transposed(layer, context.scales);
466458

467459
// For aligned (default): x is primary axis, y is computed (secondary)
468460
// For transposed: y is primary axis, x is computed (secondary)
@@ -516,12 +508,7 @@ impl GeomRenderer for LinearRenderer {
516508
let intercept_field = naming::aesthetic_column("intercept");
517509

518510
// Check orientation
519-
let is_horizontal = layer
520-
.parameters
521-
.get("orientation")
522-
.and_then(|v| v.as_str())
523-
.map(|s| s == "transposed")
524-
.unwrap_or(false);
511+
let is_horizontal = is_transposed(layer, context.scales);
525512

526513
// Get extent from appropriate axis:
527514
// - Aligned (default): extent from pos1 (x-axis), compute y from x
@@ -578,14 +565,9 @@ impl GeomRenderer for RibbonRenderer {
578565
&self,
579566
encoding: &mut Map<String, Value>,
580567
layer: &Layer,
581-
_context: &RenderContext,
568+
context: &RenderContext,
582569
) -> Result<()> {
583-
let is_horizontal = layer
584-
.parameters
585-
.get("orientation")
586-
.and_then(|v| v.as_str())
587-
.map(|s| s == "transposed")
588-
.unwrap_or(false);
570+
let is_horizontal = is_transposed(layer, context.scales);
589571

590572
// Remap min/max to primary/secondary based on orientation:
591573
// - Aligned (vertical): ymax→y, ymin→y2
@@ -662,7 +644,7 @@ impl GeomRenderer for ViolinRenderer {
662644
&self,
663645
layer_spec: &mut Value,
664646
layer: &Layer,
665-
_context: &RenderContext,
647+
context: &RenderContext,
666648
) -> Result<()> {
667649
layer_spec["mark"] = json!({
668650
"type": "line",
@@ -674,12 +656,7 @@ impl GeomRenderer for ViolinRenderer {
674656
let violin_offset = format!("[datum.{offset}, -datum.{offset}]", offset = offset_col);
675657

676658
// Read orientation from layer (already resolved during execution)
677-
let is_horizontal = layer
678-
.parameters
679-
.get("orientation")
680-
.and_then(|v| v.as_str())
681-
.map(|s| s == "transposed")
682-
.unwrap_or(false);
659+
let is_horizontal = is_transposed(layer, context.scales);
683660

684661
// Continuous axis column for order calculation:
685662
// - Vertical: pos2 (y-axis has continuous density values)
@@ -751,15 +728,10 @@ impl GeomRenderer for ViolinRenderer {
751728
&self,
752729
encoding: &mut Map<String, Value>,
753730
layer: &Layer,
754-
_context: &RenderContext,
731+
context: &RenderContext,
755732
) -> Result<()> {
756733
// Read orientation from layer (already resolved during execution)
757-
let is_horizontal = layer
758-
.parameters
759-
.get("orientation")
760-
.and_then(|v| v.as_str())
761-
.map(|s| s == "transposed")
762-
.unwrap_or(false);
734+
let is_horizontal = is_transposed(layer, context.scales);
763735

764736
// Categorical axis for detail encoding:
765737
// - Vertical: x channel (categorical groups on x-axis)
@@ -906,6 +878,7 @@ impl GeomRenderer for ErrorBarRenderer {
906878
layer: &Layer,
907879
_data_key: &str,
908880
_prepared: &PreparedData,
881+
_context: &RenderContext,
909882
) -> Result<Vec<Value>> {
910883
// Get width parameter (in points)
911884
let width = if let Some(ParameterValue::Number(num)) = layer.parameters.get("width") {
@@ -1035,16 +1008,12 @@ impl BoxplotRenderer {
10351008
layer: &Layer,
10361009
base_key: &str,
10371010
has_outliers: bool,
1011+
context: &RenderContext,
10381012
) -> Result<Vec<Value>> {
10391013
let mut layers: Vec<Value> = Vec::new();
10401014

10411015
// Read orientation from layer (already resolved during execution)
1042-
let is_horizontal = layer
1043-
.parameters
1044-
.get("orientation")
1045-
.and_then(|v| v.as_str())
1046-
.map(|s| s == "transposed")
1047-
.unwrap_or(false);
1016+
let is_horizontal = is_transposed(layer, context.scales);
10481017

10491018
// Value columns depend on orientation (after DataFrame column flip):
10501019
// - Vertical: values in pos2/pos2end (no flip)
@@ -1258,6 +1227,7 @@ impl GeomRenderer for BoxplotRenderer {
12581227
layer: &Layer,
12591228
data_key: &str,
12601229
prepared: &PreparedData,
1230+
context: &RenderContext,
12611231
) -> Result<Vec<Value>> {
12621232
let PreparedData::Composite { metadata, .. } = prepared else {
12631233
return Err(GgsqlError::InternalError(
@@ -1269,7 +1239,7 @@ impl GeomRenderer for BoxplotRenderer {
12691239
GgsqlError::InternalError("Failed to downcast boxplot metadata".to_string())
12701240
})?;
12711241

1272-
self.render_layers(prototype, layer, data_key, info.has_outliers)
1242+
self.render_layers(prototype, layer, data_key, info.has_outliers, context)
12731243
}
12741244
}
12751245

‎src/writer/vegalite/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ fn build_layers(
196196
renderer.modify_spec(&mut layer_spec, layer, &context)?;
197197

198198
// Finalize the layer (may expand into multiple layers for composite geoms)
199-
let final_layers = renderer.finalize(layer_spec, layer, data_key, prepared)?;
199+
let final_layers = renderer.finalize(layer_spec, layer, data_key, prepared, &context)?;
200200
layers.extend(final_layers);
201201
}
202202

0 commit comments

Comments
 (0)