Skip to content

Commit d684fa3

Browse files
cpsievertclaude
andauthored
fix: use utcFormat for temporal axis label comparisons (#161)
* fix: use utcFormat instead of timeFormat for temporal axis labels The Vega-Lite labelExpr for temporal scales (date, datetime, time) was using timeFormat() to compare axis tick values against ISO date keys from the RENAMING clause. timeFormat formats in the browser's local timezone, but ggsql writes all temporal data as UTC ISO strings (e.g., "2024-01-01" represents midnight UTC). In non-UTC timezones, this causes every label comparison to fail. For example, in US Central Time (UTC-6), timeFormat formats "2024-01-01" (midnight UTC) as "2023-12-31" (6 PM local), so the comparison against the key "2024-01-01" never matches. The labelExpr falls through to datum.label (Vega-Lite's default formatter), which shows time-of-day labels like "06 PM" / "07 PM" instead of the expected date labels. The fix is to use utcFormat(), which formats in UTC and matches the ISO date keys that ggsql generates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * cargo fmt --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c1979cc commit d684fa3

1 file changed

Lines changed: 77 additions & 4 deletions

File tree

src/writer/vegalite/encoding.rs

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ use super::{POINTS_TO_AREA, POINTS_TO_PIXELS};
2424
/// - Example: `"datum.label == 'A' ? 'Alpha' : datum.label == 'B' ? 'Beta' : datum.label"`
2525
///
2626
/// For temporal scales:
27-
/// - Uses `timeFormat(datum.value, 'fmt')` for comparisons
27+
/// - Uses `utcFormat(datum.value, 'fmt')` for comparisons (UTC to match our ISO date strings)
2828
/// - This is necessary because `datum.label` contains Vega-Lite's formatted label (e.g., "Jan 1, 2024")
2929
/// but our label_mapping keys are ISO format strings (e.g., "2024-01-01")
30-
/// - Example: `"timeFormat(datum.value, '%Y-%m-%d') == '2024-01-01' ? 'Q1 Start' : datum.label"`
30+
/// - Example: `"utcFormat(datum.value, '%Y-%m-%d') == '2024-01-01' ? 'Q1 Start' : datum.label"`
3131
///
3232
/// For threshold scales (binned legends):
3333
/// - The `null_key` parameter specifies which key should use `datum.label == null` instead of
@@ -43,8 +43,11 @@ pub(super) fn build_label_expr(
4343
}
4444

4545
// Build the comparison expression based on whether this is temporal
46+
// Use utcFormat (not timeFormat) because ggsql writes ISO date strings as UTC.
47+
// timeFormat uses the browser's local timezone, causing comparison mismatches
48+
// (e.g., "2024-01-01" UTC midnight becomes "2023-12-31" in US timezones).
4649
let comparison_expr = match time_format {
47-
Some(fmt) => format!("timeFormat(datum.value, '{}')", fmt),
50+
Some(fmt) => format!("utcFormat(datum.value, '{}')", fmt),
4851
None => "datum.label".to_string(),
4952
};
5053

@@ -668,7 +671,7 @@ fn apply_label_mapping_to_encoding(
668671
return;
669672
}
670673

671-
// For temporal scales, use timeFormat() to compare against ISO keys
674+
// For temporal scales, use utcFormat() to compare against ISO keys
672675
let time_format = scale
673676
.transform
674677
.as_ref()
@@ -939,3 +942,73 @@ pub(super) fn build_detail_encoding(partition_by: &[String]) -> Option<Value> {
939942
Some(json!(details))
940943
}
941944
}
945+
946+
#[cfg(test)]
947+
mod tests {
948+
use super::*;
949+
950+
#[test]
951+
fn test_build_label_expr_temporal_uses_utc_format() {
952+
// Temporal label comparisons must use utcFormat (not timeFormat) because
953+
// ggsql writes ISO date strings as UTC. timeFormat uses the browser's
954+
// local timezone, causing comparisons to fail in non-UTC timezones
955+
// (e.g., "2024-01-01" midnight UTC becomes "2023-12-31" in US Central).
956+
let mut mappings = HashMap::new();
957+
mappings.insert("2024-01-01".to_string(), Some("Jan 2024".to_string()));
958+
959+
let expr = build_label_expr(&mappings, Some("%Y-%m-%d"), None);
960+
961+
assert!(
962+
expr.contains("utcFormat("),
963+
"temporal labelExpr should use utcFormat, got: {expr}"
964+
);
965+
assert!(
966+
!expr.contains("timeFormat("),
967+
"temporal labelExpr must not use timeFormat (local tz), got: {expr}"
968+
);
969+
assert!(
970+
expr.contains("utcFormat(datum.value, '%Y-%m-%d') == '2024-01-01' ? 'Jan 2024'"),
971+
"expected correct comparison expression, got: {expr}"
972+
);
973+
}
974+
975+
#[test]
976+
fn test_build_label_expr_non_temporal_uses_datum_label() {
977+
let mut mappings = HashMap::new();
978+
mappings.insert("A".to_string(), Some("Alpha".to_string()));
979+
980+
let expr = build_label_expr(&mappings, None, None);
981+
982+
assert!(
983+
expr.contains("datum.label == 'A'"),
984+
"non-temporal should use datum.label, got: {expr}"
985+
);
986+
assert!(
987+
!expr.contains("utcFormat("),
988+
"non-temporal should not use utcFormat, got: {expr}"
989+
);
990+
}
991+
992+
#[test]
993+
fn test_build_label_expr_fallback() {
994+
let mappings = HashMap::new();
995+
let expr = build_label_expr(&mappings, Some("%Y-%m-%d"), None);
996+
assert_eq!(
997+
expr, "datum.label",
998+
"empty mappings should fall back to datum.label"
999+
);
1000+
}
1001+
1002+
#[test]
1003+
fn test_build_label_expr_null_suppression() {
1004+
let mut mappings = HashMap::new();
1005+
mappings.insert("2024-06-01".to_string(), None); // suppress label
1006+
1007+
let expr = build_label_expr(&mappings, Some("%Y-%m-%d"), None);
1008+
1009+
assert!(
1010+
expr.contains("? ''"),
1011+
"None mapping should suppress label (empty string), got: {expr}"
1012+
);
1013+
}
1014+
}

0 commit comments

Comments
 (0)