simplify flipping

Author: thomasp85

src/execute/layer.rs

@@ -5,7 +5,7 @@
 
 use crate::plot::aesthetic::{is_positional_aesthetic, AestheticContext};
 use crate::plot::layer::is_transposed;
-use crate::plot::layer::orientation::{flip_mappings, resolve_orientation};
+use crate::plot::layer::orientation::{flip_positional_aesthetics, resolve_orientation};
 use crate::plot::{
     AestheticValue, DefaultAestheticValue, Layer, ParameterValue, Scale, Schema, SqlTypeNames,
     StatResult,
@@ -356,7 +356,7 @@ pub fn apply_layer_transforms<F>(
 where
     F: Fn(&str) -> Result<DataFrame>,
 {
-    use crate::plot::layer::orientation::{flip_mappings, flip_remappings};
+    use crate::plot::layer::orientation::flip_positional_aesthetics;
 
     // Clone order_by early to avoid borrow conflicts
     let order_by = layer.order_by.clone();
@@ -433,7 +433,7 @@ where
     // 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);
+        flip_positional_aesthetics(&mut layer.remappings.aesthetics);
     }
 
     // Apply literal default remappings from geom defaults (e.g., y2 => 0.0 for bar baseline).
@@ -580,10 +580,10 @@ where
     // 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);
+        flip_positional_aesthetics(&mut layer.mappings.aesthetics);
 
         // Normalize mapping column names to match their aesthetic keys.
-        // After flip_mappings, pos1 might point to __ggsql_aes_pos2__ (and vice versa).
+        // After flipping, 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);
@@ -601,11 +601,11 @@ where
 
 /// 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.
+/// After flipping positional aesthetics, 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.
+/// This should be called after flipping during flip-back.
 /// The DataFrame columns should be renamed correspondingly using `flip_dataframe_columns`.
 fn normalize_mapping_column_names(layer: &mut Layer) {
     // Collect the aesthetics to update (to avoid borrowing issues)
@@ -710,7 +710,7 @@ pub fn resolve_orientations(
             ParameterValue::String(orientation.to_string()),
         );
         if is_transposed(layer) {
-            flip_mappings(layer);
+            flip_positional_aesthetics(&mut layer.mappings.aesthetics);
             // Also flip column names in type_info to match the flipped mappings
             if layer_idx < layer_type_info.len() {
                 for (name, _, _) in &mut layer_type_info[layer_idx] {

src/execute/mod.rs

@@ -1165,7 +1165,9 @@ pub fn prepare_data_with_reader<R: Reader>(query: &str, reader: &R) -> Result<Pr
             // (which uses remapping keys to create mapping entries).
             // Phase 4.5 will then flip the DataFrame columns to match.
             if is_transposed(l) {
-                crate::plot::layer::orientation::flip_remappings(l);
+                crate::plot::layer::orientation::flip_positional_aesthetics(
+                    &mut l.remappings.aesthetics,
+                );
             }
 
             // Update layer mappings for all layers (even if data shared)

src/plot/layer/orientation.rs

@@ -26,7 +26,7 @@
 use super::geom::GeomType;
 use super::Layer;
 use crate::plot::scale::ScaleTypeKind;
-use crate::plot::{Mappings, Scale};
+use crate::plot::{AestheticValue, Mappings, Scale};
 
 /// Orientation value for aligned/vertical orientation.
 pub const ALIGNED: &str = "aligned";
@@ -215,7 +215,7 @@ fn is_discrete_scale(scale: &Scale) -> bool {
     })
 }
 
-/// Swap positional aesthetic pairs in layer mappings.
+/// Swap positional aesthetic pairs in an aesthetics map.
 ///
 /// Swaps the following pairs:
 /// - pos1 ↔ pos2
@@ -224,52 +224,27 @@ fn is_discrete_scale(scale: &Scale) -> bool {
 /// - pos1end ↔ pos2end
 /// - pos1offset ↔ pos2offset
 ///
-/// 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 = [
+/// Used for both mappings and remappings when handling transposed orientation.
+pub fn flip_positional_aesthetics(
+    aesthetics: &mut std::collections::HashMap<String, AestheticValue>,
+) {
+    const PAIRS: [(&str, &str); 5] = [
         ("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);
+    for (a, b) in PAIRS {
+        let val_a = aesthetics.remove(a);
+        let val_b = aesthetics.remove(b);
 
         if let Some(v) = val_a {
-            layer.mappings.aesthetics.insert(b.to_string(), v);
+            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);
+            aesthetics.insert(a.to_string(), v);
         }
     }
 }
@@ -411,7 +386,7 @@ mod tests {
     }
 
     #[test]
-    fn test_flip_mappings() {
+    fn test_flip_positional_aesthetics() {
         let mut layer = Layer::new(Geom::bar());
         layer.mappings.insert(
             "pos1".to_string(),
@@ -426,7 +401,7 @@ mod tests {
             AestheticValue::standard_column("x2".to_string()),
         );
 
-        flip_mappings(&mut layer);
+        flip_positional_aesthetics(&mut layer.mappings.aesthetics);
 
         // pos1 ↔ pos2
         assert_eq!(
@@ -446,23 +421,23 @@ mod tests {
     }
 
     #[test]
-    fn test_flip_mappings_empty() {
+    fn test_flip_positional_aesthetics_empty() {
         let mut layer = Layer::new(Geom::point());
         // No crash with empty mappings
-        flip_mappings(&mut layer);
+        flip_positional_aesthetics(&mut layer.mappings.aesthetics);
         assert!(layer.mappings.aesthetics.is_empty());
     }
 
     #[test]
-    fn test_flip_mappings_partial() {
+    fn test_flip_positional_aesthetics_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);
+        flip_positional_aesthetics(&mut layer.mappings.aesthetics);
 
         // pos1 moves to pos2
         assert!(layer.mappings.get("pos1").is_none());