Skip to content

Commit a560c99

Browse files
authored
Guide training bugfix (#5428)
* Ensure scales/aesthetics are parallel * Adapt tests * Add argument names
1 parent 89204bc commit a560c99

File tree

3 files changed

+22
-21
lines changed

3 files changed

+22
-21
lines changed

R/coord-.R

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,17 +97,18 @@ Coord <- ggproto("Coord",
9797
aesthetics <- c("x", "y", "x.sec", "y.sec")
9898
names(aesthetics) <- aesthetics
9999
is_sec <- grepl("sec$", aesthetics)
100+
scales <- panel_params[aesthetics]
100101

101102
# Do guide setup
102103
guides <- guides$setup(
103-
panel_params, aesthetics,
104+
scales, aesthetics,
104105
default = params$guide_default %||% guide_axis(),
105106
missing = params$guide_missing %||% guide_none()
106107
)
107108
guide_params <- guides$get_params(aesthetics)
108109

109110
# Resolve positions
110-
scale_position <- lapply(panel_params[aesthetics], `[[`, "position")
111+
scale_position <- lapply(scales, `[[`, "position")
111112
guide_position <- lapply(guide_params, `[[`, "position")
112113
guide_position[!is_sec] <- Map(
113114
function(guide, scale) guide %|W|% scale,

R/guides-.R

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -301,12 +301,19 @@ Guides <- ggproto(
301301
horizontal = c("center", "top")
302302
)
303303

304-
# Setup and train on scales
304+
# Extract the non-position scales
305305
scales <- scales$non_position_scales()$scales
306306
if (length(scales) == 0) {
307307
return(no_guides)
308308
}
309-
guides <- self$setup(scales)
309+
310+
# Ensure a 1:1 mapping between aesthetics and scales
311+
aesthetics <- lapply(scales, `[[`, "aesthetics")
312+
scales <- rep.int(scales, lengths(aesthetics))
313+
aesthetics <- unlist(aesthetics, recursive = FALSE, use.names = FALSE)
314+
315+
# Setup and train scales
316+
guides <- self$setup(scales, aesthetics = aesthetics)
310317
guides$train(scales, theme$legend.direction, labels)
311318
if (length(guides$guides) == 0) {
312319
return(no_guides)
@@ -343,28 +350,16 @@ Guides <- ggproto(
343350
default = self$missing,
344351
missing = self$missing
345352
) {
346-
347-
if (is.null(aesthetics)) {
348-
# Aesthetics from scale, as in non-position guides
349-
aesthetics <- lapply(scales, `[[`, "aesthetics")
350-
scale_idx <- rep(seq_along(scales), lengths(aesthetics))
351-
aesthetics <- unlist(aesthetics, FALSE, FALSE)
352-
} else {
353-
# Scale based on aesthetics, as in position guides
354-
scale_idx <- seq_along(scales)[match(aesthetics, names(scales))]
355-
}
356-
357353
guides <- self$guides
358354

359355
# For every aesthetic-scale combination, find and validate guide
360-
new_guides <- lapply(seq_along(scale_idx), function(i) {
361-
idx <- scale_idx[i]
356+
new_guides <- lapply(seq_along(scales), function(idx) {
362357

363358
# Find guide for aesthetic-scale combination
364359
# Hierarchy is in the order:
365360
# plot + guides(XXX) + scale_ZZZ(guide = XXX) > default(i.e., legend)
366361
guide <- resolve_guide(
367-
aesthetic = aesthetics[i],
362+
aesthetic = aesthetics[idx],
368363
scale = scales[[idx]],
369364
guides = guides,
370365
default = default,

tests/testthat/test-guides.R

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,15 @@ test_that("guide merging for guide_legend() works as expected", {
178178
scales <- scales_list()
179179
scales$add(scale1)
180180
scales$add(scale2)
181+
scales <- scales$scales
182+
183+
aesthetics <- lapply(scales, `[[`, "aesthetics")
184+
scales <- rep.int(scales, lengths(aesthetics))
185+
aesthetics <- unlist(aesthetics, FALSE, FALSE)
181186

182187
guides <- guides_list(NULL)
183-
guides <- guides$setup(scales$scales)
184-
guides$train(scales$scales, "vertical", labs())
188+
guides <- guides$setup(scales, aesthetics)
189+
guides$train(scales, "vertical", labs())
185190
guides$merge()
186191
guides$params
187192
}
@@ -279,7 +284,7 @@ test_that("legend reverse argument reverses the key", {
279284
scale$train(LETTERS[1:4])
280285

281286
guides <- guides_list(NULL)
282-
guides <- guides$setup(list(scale))
287+
guides <- guides$setup(list(scale), "colour")
283288

284289
guides$params[[1]]$reverse <- FALSE
285290
guides$train(list(scale), "horizontal", labels = labs())

0 commit comments

Comments
 (0)