Skip to content

Commit 5f6d3c5

Browse files
authored
Domesticate NULL aesthetic labels (#6644)
* labels derived from NULL aesthetics serve only as fallback labels * catch tests where this occurs * add news bullet * atomic vectors also produce fallback labels * move `make_labels()` helper to file where it is used * update test * fix guide order in unrelated test
1 parent ec58f51 commit 5f6d3c5

File tree

6 files changed

+34
-25
lines changed

6 files changed

+34
-25
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
not passed as the `name` argument (@teunbrand, #6623)
2323
* Fixed issue where vectorised `arrow()`s caused errors in drawing the
2424
legend glyphs (@teunbrand, #6594)
25+
* Fixed regression where `NULL`-aesthetics contributed to plot labels too
26+
insistently. Now they contribute only as fallback labels (@teunbrand, #6616)
2527

2628
# ggplot2 4.0.0
2729

R/aes-evaluation.R

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -346,26 +346,6 @@ strip_stage <- function(expr) {
346346
}
347347
}
348348

349-
# Convert aesthetic mapping into text labels
350-
make_labels <- function(mapping) {
351-
default_label <- function(aesthetic, mapping) {
352-
# e.g., geom_smooth(aes(colour = "loess")) or aes(y = NULL)
353-
if (is.null(mapping) || is.atomic(mapping)) {
354-
return(aesthetic)
355-
}
356-
mapping <- strip_stage(mapping)
357-
mapping <- strip_dots(mapping, strip_pronoun = TRUE)
358-
if (is_quosure(mapping) && quo_is_symbol(mapping)) {
359-
name <- as_string(quo_get_expr(mapping))
360-
} else {
361-
name <- quo_text(mapping)
362-
name <- gsub("\n.*$", "...", name)
363-
}
364-
name
365-
}
366-
Map(default_label, names(mapping), mapping)
367-
}
368-
369349
eval_aesthetics <- function(aesthetics, data, mask = NULL) {
370350

371351
env <- child_env(base_env())

R/labels.R

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,26 @@ setup_plot_labels <- function(plot, layers, data) {
120120
labs(!!!defaults(plot_labels, labels))
121121
}
122122

123+
# Convert aesthetic mapping into text labels
124+
make_labels <- function(mapping) {
125+
default_label <- function(aesthetic, mapping) {
126+
# e.g., geom_smooth(aes(colour = "loess")) or aes(y = NULL)
127+
if (is.null(mapping) || is.atomic(mapping)) {
128+
return(structure(aesthetic, fallback = TRUE))
129+
}
130+
mapping <- strip_stage(mapping)
131+
mapping <- strip_dots(mapping, strip_pronoun = TRUE)
132+
if (is_quosure(mapping) && quo_is_symbol(mapping)) {
133+
name <- as_string(quo_get_expr(mapping))
134+
} else {
135+
name <- quo_text(mapping)
136+
name <- gsub("\n.*$", "...", name)
137+
}
138+
name
139+
}
140+
Map(default_label, names(mapping), mapping)
141+
}
142+
123143
#' Modify axis, legend, and plot labels
124144
#'
125145
#' Good labels are critical for making your plots accessible to a wider

tests/testthat/test-aes-calculated.R

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,11 @@ test_that("make_labels() deparses mappings properly", {
4545
x_lab <- make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x))$x
4646
expect_length(x_lab, 1L)
4747
expect_match(x_lab, "...$")
48-
# if the mapping is a literal or NULL, the aesthetics is used
49-
expect_identical(make_labels(aes(x = 1)), list(x = "x"))
50-
expect_identical(make_labels(aes(x = NULL)), list(x = "x"))
48+
fallback <- list(x = structure("x", fallback = TRUE))
49+
# if the mapping is a literal or NULL, the aesthetics is used as fallback
50+
expect_identical(make_labels(aes(x = 1)), fallback)
51+
# NULL labels should only be used as fallback labels
52+
expect_identical(make_labels(aes(x = NULL)), fallback)
5153
})
5254

5355
test_that("staged aesthetics warn appropriately for duplicated names", {

tests/testthat/test-aes.R

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ test_that("quosures are squashed when creating default label for a mapping", {
102102
test_that("labelling doesn't cause error if aesthetic is NULL", {
103103
p <- ggplot(mtcars) + aes(x = NULL)
104104
labels <- ggplot_build(p)@plot@labels
105-
expect_identical(labels$x, "x")
105+
# NULL labels should only be used as fallback labels
106+
expect_identical(labels$x, structure("x", fallback = TRUE))
106107
})
107108

108109
test_that("aes standardises aesthetic names", {

tests/testthat/test-draw-key.R

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ test_that("keep_draw_key", {
110110
aes(colour = "line", alpha = "line"),
111111
show.legend = c("colour" = NA, alpha = TRUE)
112112
) +
113-
suppressWarnings(scale_alpha_discrete())
113+
suppressWarnings(scale_alpha_discrete()) +
114+
guides(
115+
alpha = guide_legend(order = 1),
116+
colour = guide_legend(order = 2)
117+
)
114118

115119
expect_doppelganger("appropriate colour key with alpha key as lines", p)
116120

0 commit comments

Comments
 (0)