diff --git a/NEWS.md b/NEWS.md index d1818f3f0a..2d54ba1b84 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* (internal) guide building is now part of `ggplot_build()` instead of + `ggplot_gtable()` to allow guides to observe unmapped data (#5483). + * `geom_violin()` gains a `bounds` argument analogous to `geom_density()`s (@eliocamp, #5493). * Legend titles no longer take up space if they've been removed by setting diff --git a/R/coord-cartesian-.R b/R/coord-cartesian-.R index 10ab907147..95e125cdc0 100644 --- a/R/coord-cartesian-.R +++ b/R/coord-cartesian-.R @@ -151,5 +151,5 @@ panel_guides_grob <- function(guides, position, theme) { return(zeroGrob()) } pair <- guides$get_position(position) - pair$guide$draw(theme, pair$params) + pair$guide$draw(theme, params = pair$params) } diff --git a/R/guide-.R b/R/guide-.R index a3f449b9ed..bdf360db8e 100644 --- a/R/guide-.R +++ b/R/guide-.R @@ -117,9 +117,12 @@ new_guide <- function(..., available_aes = "any", super) { #' `params$hash`. This ensures that e.g. `guide_legend()` can display both #' `shape` and `colour` in the same guide. #' -#' - `get_layer_key()` Extract information from layers. This can be used to -#' check that the guide's aesthetic is actually in use, or to gather -#' information about how legend keys should be displayed. +#' - `process_layers()` Extract information from layers. This acts mostly +#' as a filter for which layers to include and these are then (typically) +#' forwarded to `get_layer_key()`. +#' +#' - `get_layer_key()` This can be used to gather information about how legend +#' keys should be displayed. #' #' - `setup_params()` Set up parameters at the beginning of drawing stages. #' It can be used to overrule user-supplied parameters or perform checks on @@ -253,7 +256,11 @@ Guide <- ggproto( # Function for extracting information from the layers. # Mostly applies to `guide_legend()` and `guide_binned()` - get_layer_key = function(params, layers) { + process_layers = function(self, params, layers, data = NULL) { + self$get_layer_key(params, layers, data) + }, + + get_layer_key = function(params, layers, data = NULL) { return(params) }, @@ -280,11 +287,14 @@ Guide <- ggproto( # Main drawing function that organises more specialised aspects of guide # drawing. - draw = function(self, theme, params = self$params) { + draw = function(self, theme, position = NULL, direction = NULL, + params = self$params) { key <- params$key # Setup parameters and theme + params$position <- params$position %||% position + params$direction <- params$direction %||% direction params <- self$setup_params(params) elems <- self$setup_elements(params, self$elements, theme) elems <- self$override_elements(params, elems, theme) diff --git a/R/guide-axis.R b/R/guide-axis.R index 6f15c1f23e..8e23b155a0 100644 --- a/R/guide-axis.R +++ b/R/guide-axis.R @@ -486,7 +486,7 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, !!aes := c(0, 1), !!opp := opp_value ) - guide$draw(theme, params) + guide$draw(theme, params = params) } draw_axis_labels <- function(break_positions, break_labels, label_element, is_vertical, diff --git a/R/guide-bins.R b/R/guide-bins.R index 63c75bd0bd..f20adee759 100644 --- a/R/guide-bins.R +++ b/R/guide-bins.R @@ -298,15 +298,15 @@ GuideBins <- ggproto( params$title <- scale$make_title( params$title %|W|% scale$name %|W|% title ) + params$key <- key + params + }, + + setup_params = function(params) { params$direction <- arg_match0( - params$direction %||% direction, + params$direction, c("horizontal", "vertical"), arg_nm = "direction" ) - if (params$direction == "vertical") { - key$.value <- 1 - key$.value - } - - params$key <- key valid_label_pos <- switch( params$direction, "horizontal" = c("bottom", "top"), @@ -320,10 +320,6 @@ GuideBins <- ggproto( "not {.val {params$label.position}}." )) } - params - }, - - setup_params = function(params) { params <- GuideLegend$setup_params(params) params$byrow <- FALSE params$rejust_labels <- FALSE @@ -345,10 +341,15 @@ GuideBins <- ggproto( } key$.label[c(1, n_labels)[!params$show.limits]] <- "" - just <- if (params$direction == "horizontal") { - elements$text$vjust - } else { - elements$text$hjust + just <- switch( + params$direction, + horizontal = elements$text$vjust, + vertical = elements$text$hjust, + 0.5 + ) + + if (params$direction == "vertical") { + key$.value <- 1 - key$.value } list(labels = flip_element_grob( @@ -363,6 +364,9 @@ GuideBins <- ggproto( }, build_ticks = function(key, elements, params, position = params$position) { + if (params$direction == "vertical") { + key$.value <- 1 - key$.value + } key$.value[c(1, nrow(key))[!params$show.limits]] <- NA Guide$build_ticks(key$.value, elements, params, params$label.position) }, diff --git a/R/guide-colorbar.R b/R/guide-colorbar.R index 4917679ebf..7e71eaba0c 100644 --- a/R/guide-colorbar.R +++ b/R/guide-colorbar.R @@ -344,27 +344,10 @@ GuideColourbar <- ggproto( }, extract_params = function(scale, params, - title = waiver(), direction = "vertical", ...) { + title = waiver(), ...) { params$title <- scale$make_title( params$title %|W|% scale$name %|W|% title ) - params$direction <- arg_match0( - params$direction %||% direction, - c("horizontal", "vertical"), arg_nm = "direction" - ) - valid_label_pos <- switch( - params$direction, - "horizontal" = c("bottom", "top"), - "vertical" = c("right", "left") - ) - params$label.position <- params$label.position %||% valid_label_pos[1] - if (!params$label.position %in% valid_label_pos) { - cli::cli_abort(paste0( - "When {.arg direction} is {.val {params$direction}}, ", - "{.arg label.position} must be one of {.or {.val {valid_label_pos}}}, ", - "not {.val {params$label.position}}." - )) - } limits <- c(params$decor$value[1], params$decor$value[nrow(params$decor)]) params$key$.value <- rescale( @@ -381,27 +364,28 @@ GuideColourbar <- ggproto( return(list(guide = self, params = params)) }, - get_layer_key = function(params, layers) { - - guide_layers <- lapply(layers, function(layer) { - - matched_aes <- matched_aes(layer, params) - - # Check if this layer should be included - if (include_layer_in_guide(layer, matched_aes)) { - layer - } else { - NULL - } - }) - - if (length(compact(guide_layers)) == 0) { - return(NULL) - } - return(params) + get_layer_key = function(params, layers, data = NULL) { + params }, setup_params = function(params) { + params$direction <- arg_match0( + params$direction, + c("horizontal", "vertical"), arg_nm = "direction" + ) + valid_label_pos <- switch( + params$direction, + "horizontal" = c("bottom", "top"), + "vertical" = c("right", "left") + ) + params$label.position <- params$label.position %||% valid_label_pos[1] + if (!params$label.position %in% valid_label_pos) { + cli::cli_abort(paste0( + "When {.arg direction} is {.val {params$direction}}, ", + "{.arg label.position} must be one of {.or {.val {valid_label_pos}}}, ", + "not {.val {params$label.position}}." + )) + } params$title.position <- arg_match0( params$title.position %||% switch(params$direction, vertical = "top", horizontal = "left"), diff --git a/R/guide-legend.R b/R/guide-legend.R index 087e3e6fef..341bee47c8 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -244,7 +244,7 @@ GuideLegend <- ggproto( available_aes = "any", - hashables = exprs(title, key$.label, direction, name), + hashables = exprs(title, key$.label, name), elements = list( background = "legend.background", @@ -260,14 +260,10 @@ GuideLegend <- ggproto( ), extract_params = function(scale, params, - title = waiver(), direction = NULL, ...) { + title = waiver(), ...) { params$title <- scale$make_title( params$title %|W|% scale$name %|W|% title ) - params$direction <- arg_match0( - params$direction %||% direction, - c("horizontal", "vertical"), arg_nm = "direction" - ) if (isTRUE(params$reverse %||% FALSE)) { params$key <- params$key[nrow(params$key):1, , drop = FALSE] } @@ -291,17 +287,26 @@ GuideLegend <- ggproto( }, # Arrange common data for vertical and horizontal legends - get_layer_key = function(params, layers) { + process_layers = function(self, params, layers, data = NULL) { + + include <- vapply(layers, function(layer) { + aes <- matched_aes(layer, params) + include_layer_in_guide(layer, aes) + }, logical(1)) + + if (!any(include)) { + return(NULL) + } + + self$get_layer_key(params, layers[include], data[include]) + }, + + get_layer_key = function(params, layers, data) { decor <- lapply(layers, function(layer) { matched_aes <- matched_aes(layer, params) - # Check if this layer should be included - if (!include_layer_in_guide(layer, matched_aes)) { - return(NULL) - } - if (length(matched_aes) > 0) { # Filter out aesthetics that can't be applied to the legend n <- lengths(layer$aes_params, use.names = FALSE) @@ -338,14 +343,15 @@ GuideLegend <- ggproto( # Remove NULL geoms params$decor <- compact(decor) - - if (length(params$decor) == 0) { - return(NULL) - } return(params) }, setup_params = function(params) { + params$direction <- arg_match0( + params$direction %||% direction, + c("horizontal", "vertical"), arg_nm = "direction" + ) + if ("title.position" %in% names(params)) { params$title.position <- arg_match0( params$title.position %||% diff --git a/R/guide-none.R b/R/guide-none.R index ae26a8a1e9..5c0b2d35e2 100644 --- a/R/guide-none.R +++ b/R/guide-none.R @@ -35,7 +35,7 @@ GuideNone <- ggproto( }, # Draw nothing - draw = function(self, params, theme) { + draw = function(self, ...) { zeroGrob() } ) diff --git a/R/guide-old.R b/R/guide-old.R index 2320b0bbf2..b2a137fffd 100644 --- a/R/guide-old.R +++ b/R/guide-old.R @@ -90,7 +90,7 @@ GuideOld <- ggproto( train = function(self, params, scale, aesthetic = NULL, title = waiver(), direction = NULL) { params$title <- scale$make_title(params$title %|W|% scale$name %|W|% title) - params$direction <- params$direction %||% direction + params$direction <- params$direction %||% direction %||% "vertical" params <- guide_train(params, scale, aesthetic) params }, @@ -103,13 +103,14 @@ GuideOld <- ggproto( guide_transform(params, coord, panel_params) }, - get_layer_key = function(params, layers) { + process_layers = function(self, params, layers, data = NULL) { guide_geom(params, layers, default_mapping = NULL) }, - draw = function(self, theme, params) { + draw = function(self, theme, position = NULL, direction = NULL, params) { + params$direction <- params$direction %||% direction %||% "placeholder" params$title.position <- params$title.position %||% switch( - params$direction %||% "placeholder", + params$direction, vertical = "top", horizontal = "left", NULL ) diff --git a/R/guides-.R b/R/guides-.R index 2117edda62..76bac43de0 100644 --- a/R/guides-.R +++ b/R/guides-.R @@ -278,28 +278,10 @@ Guides <- ggproto( # 5. Guides$assemble() # arrange all guide grobs - build = function(self, scales, layers, default_mapping, - position, theme, labels) { + build = function(self, scales, layers, labels, layer_data) { - position <- legend_position(position) - no_guides <- zeroGrob() - if (position == "none") { - return(no_guides) - } - - theme$legend.key.width <- theme$legend.key.width %||% theme$legend.key.size - theme$legend.key.height <- theme$legend.key.height %||% theme$legend.key.size - - - default_direction <- if (position == "inside") "vertical" else position - theme$legend.box <- theme$legend.box %||% default_direction - theme$legend.direction <- theme$legend.direction %||% default_direction - theme$legend.box.just <- theme$legend.box.just %||% switch( - position, - inside = c("center", "center"), - vertical = c("left", "top"), - horizontal = c("center", "top") - ) + # Empty guides list + no_guides <- guides_list() # Extract the non-position scales scales <- scales$non_position_scales()$scales @@ -314,21 +296,19 @@ Guides <- ggproto( # Setup and train scales guides <- self$setup(scales, aesthetics = aesthetics) - guides$train(scales, theme$legend.direction, labels) + guides$train(scales, labels) + if (length(guides$guides) == 0) { return(no_guides) } # Merge and process layers guides$merge() - guides$process_layers(layers) + guides$process_layers(layers, layer_data) if (length(guides$guides) == 0) { return(no_guides) } - - # Draw and assemble - grobs <- guides$draw(theme) - guides$assemble(grobs, theme) + guides }, # Setup routine for resolving and validating guides based on paired scales. @@ -409,14 +389,13 @@ Guides <- ggproto( # Loop over every guide-scale combination to perform training # A strong assumption here is that `scales` is parallel to the guides - train = function(self, scales, direction, labels) { + train = function(self, scales, labels) { params <- Map( function(guide, param, scale, aes) { guide$train( param, scale, aes, - title = labels[[aes]], - direction = direction + title = labels[[aes]] ) }, guide = self$guides, @@ -468,9 +447,9 @@ Guides <- ggproto( }, # Loop over guides to let them extract information from layers - process_layers = function(self, layers) { + process_layers = function(self, layers, data = NULL) { self$params <- Map( - function(guide, param) guide$get_layer_key(param, layers), + function(guide, param) guide$process_layers(param, layers, data), guide = self$guides, param = self$params ) @@ -480,16 +459,43 @@ Guides <- ggproto( }, # Loop over every guide, let them draw their grobs - draw = function(self, theme) { + draw = function(self, theme, position, direction) { Map( - function(guide, params) guide$draw(theme, params), + function(guide, params) guide$draw(theme, position, direction, params), guide = self$guides, params = self$params ) }, # Combining multiple guides in a guide box - assemble = function(grobs, theme) { + assemble = function(self, theme, position) { + + if (length(self$guides) < 1) { + return(zeroGrob()) + } + + position <- legend_position(position) + if (position == "none") { + return(zeroGrob()) + } + default_direction <- if (position == "inside") "vertical" else position + + theme$legend.key.width <- theme$legend.key.width %||% theme$legend.key.size + theme$legend.key.height <- theme$legend.key.height %||% theme$legend.key.size + theme$legend.box <- theme$legend.box %||% default_direction + theme$legend.direction <- theme$legend.direction %||% default_direction + theme$legend.box.just <- theme$legend.box.just %||% switch( + position, + inside = c("center", "center"), + vertical = c("left", "top"), + horizontal = c("center", "top") + ) + + grobs <- self$draw(theme, position, default_direction) + if (length(grobs) < 1) { + return(zeroGrob()) + } + # Set spacing theme$legend.spacing <- theme$legend.spacing %||% unit(0.5, "lines") theme$legend.spacing.y <- theme$legend.spacing.y %||% theme$legend.spacing diff --git a/R/plot-build.R b/R/plot-build.R index 2c1695e350..10ffaa9ae5 100644 --- a/R/plot-build.R +++ b/R/plot-build.R @@ -84,11 +84,18 @@ ggplot_build.ggplot <- function(plot) { layout$setup_panel_params() data <- layout$map_position(data) - # Train and map non-position scales + # Hand off position guides to layout + layout$setup_panel_guides(plot$guides, plot$layers) + + # Train and map non-position scales and guides npscales <- scales$non_position_scales() if (npscales$n() > 0) { lapply(data, npscales$train_df) + plot$guides <- plot$guides$build(npscales, plot$layers, plot$labels, data) data <- lapply(data, npscales$map_df) + } else { + # Assign empty guides if there are no non-position scales + plot$guides <- guides_list() } # Fill in defaults etc. @@ -168,7 +175,6 @@ ggplot_gtable.ggplot_built <- function(data) { geom_grobs <- by_layer(function(l, d) l$draw_geom(d, layout), plot$layers, data, "converting geom to grob") - layout$setup_panel_guides(plot$guides, plot$layers) plot_table <- layout$render(geom_grobs, data, theme, plot$labels) # Legends @@ -177,9 +183,7 @@ ggplot_gtable.ggplot_built <- function(data) { position <- "manual" } - legend_box <- plot$guides$build( - plot$scales, plot$layers, plot$mapping, position, theme, plot$labels - ) + legend_box <- plot$guides$assemble(theme, position) if (is.zero(legend_box)) { position <- "none" diff --git a/tests/testthat/test-draw-key.R b/tests/testthat/test-draw-key.R index aeba592a6c..340ffb4c6d 100644 --- a/tests/testthat/test-draw-key.R +++ b/tests/testthat/test-draw-key.R @@ -7,14 +7,16 @@ test_that("alternative key glyphs work", { expect_doppelganger("time series and polygon key glyphs", ggplot(df, aes(x, y)) + geom_line(aes(color = "line"), key_glyph = "timeseries") + - geom_point(aes(fill = z), pch = 21, size = 3, key_glyph = "polygon") + geom_point(aes(fill = z), pch = 21, size = 3, key_glyph = "polygon") + + guides(fill = guide_legend(order = 1)) ) # specify key glyph by function expect_doppelganger("rectangle and dotplot key glyphs", ggplot(df, aes(x, y)) + geom_line(aes(color = "line"), key_glyph = draw_key_rect) + - geom_point(aes(fill = z), pch = 21, size = 3, stroke = 2, key_glyph = draw_key_dotplot) + geom_point(aes(fill = z), pch = 21, size = 3, stroke = 2, key_glyph = draw_key_dotplot) + + guides(fill = guide_legend(order = 1)) ) }) @@ -43,11 +45,13 @@ test_that("horizontal key glyphs work", { expect_doppelganger("horizontal boxplot and crossbar", p + geom_boxplot(aes(y = group1, color = group1), stat = "identity") + - geom_crossbar(aes(y = group2, fill = group2)) + geom_crossbar(aes(y = group2, fill = group2)) + + guides(color = guide_legend(order = 1)) ) expect_doppelganger("horizontal linerange and pointrange", p + geom_linerange(aes(y = group1, color = group1)) + - geom_pointrange(aes(y = group2, shape = group2)) + geom_pointrange(aes(y = group2, shape = group2)) + + guides(color = guide_legend(order = 1)) ) }) diff --git a/tests/testthat/test-geom-dotplot.R b/tests/testthat/test-geom-dotplot.R index eedd4ba4e3..a095158937 100644 --- a/tests/testthat/test-geom-dotplot.R +++ b/tests/testthat/test-geom-dotplot.R @@ -196,7 +196,8 @@ test_that("geom_dotplot draws correctly", { ) ) + geom_dotplot(binwidth = .4, fill = "red", col = "blue") + - continuous_scale("stroke", palette = function(x) scales::rescale(x, to = c(1, 6))) + continuous_scale("stroke", palette = function(x) scales::rescale(x, to = c(1, 6))) + + guides(linetype = guide_legend(order = 1)) ) # Stacking groups diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 6f7e241c92..b00fe359c6 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -107,8 +107,7 @@ test_that("a warning is generated when guides are drawn at a location that doesn plot <- ggplot(mpg, aes(class, hwy)) + geom_point() + scale_y_continuous(guide = guide_axis(position = "top")) - built <- expect_silent(ggplot_build(plot)) - expect_warning(ggplot_gtable(built), "Position guide is perpendicular") + expect_warning(ggplot_build(plot), "Position guide is perpendicular") }) test_that("a warning is not generated when a guide is specified with duplicate breaks", { @@ -152,22 +151,17 @@ test_that("guide_none() can be used in non-position scales", { guides <- guides$build( plot$scales, plot$layers, - plot$mapping, - "right", - theme_gray(), plot$labels ) - expect_identical(guides, zeroGrob()) + expect_length(guides$guides, 0) }) test_that("Using non-position guides for position scales results in an informative error", { p <- ggplot(mpg, aes(cty, hwy)) + geom_point() + scale_x_continuous(guide = guide_legend()) - - built <- ggplot_build(p) - expect_snapshot_warning(ggplot_gtable(built)) + expect_snapshot_warning(ggplot_build(p)) }) test_that("guide merging for guide_legend() works as expected", { @@ -186,7 +180,7 @@ test_that("guide merging for guide_legend() works as expected", { guides <- guides_list(NULL) guides <- guides$setup(scales, aesthetics) - guides$train(scales, "vertical", labs()) + guides$train(scales, labs()) guides$merge() guides$params } @@ -287,11 +281,11 @@ test_that("legend reverse argument reverses the key", { guides <- guides$setup(list(scale), "colour") guides$params[[1]]$reverse <- FALSE - guides$train(list(scale), "horizontal", labels = labs()) + guides$train(list(scale), labels = labs()) fwd <- guides$get_params(1)$key guides$params[[1]]$reverse <- TRUE - guides$train(list(scale), "horizontal", labels = labs()) + guides$train(list(scale), labels = labs()) rev <- guides$get_params(1)$key expect_equal(fwd$colour, rev(rev$colour)) @@ -306,10 +300,10 @@ test_that("guide_coloursteps and guide_bins return ordered breaks", { key <- g$train(scale = scale, aesthetic = "colour")$key expect_true(all(diff(key$.value) > 0)) - # Bins guide is decreasing order + # Bins guide is increasing order g <- guide_bins() - key <- g$train(scale = scale, aesthetics = "colour", direction = "vertical")$key - expect_true(all(diff(key$.value) < 0)) + key <- g$train(scale = scale, aesthetics = "colour")$key + expect_true(all(diff(key$.value) > 0)) }) @@ -605,7 +599,7 @@ test_that("guides are positioned correctly", { dat <- data_frame(x = LETTERS[1:3], y = 1) p2 <- ggplot(dat, aes(x, y, fill = x, colour = 1:3)) + geom_bar(stat = "identity") + - guides(color = "colorbar") + + guides(color = guide_colourbar(order = 1)) + theme_test() + theme(legend.background = element_rect(colour = "black")) @@ -841,8 +835,7 @@ test_that("a warning is generated when guides( = FALSE) is specified", { # warn on scale_*(guide = FALSE) p <- ggplot(df, aes(x, y, colour = x)) + scale_colour_continuous(guide = FALSE) - built <- expect_silent(ggplot_build(p)) - expect_snapshot_warning(ggplot_gtable(built)) + expect_snapshot_warning(ggplot_build(p)) }) test_that("guides() warns if unnamed guides are provided", {