From 1cee4c4d7e5fd4d862cdadf0f9e4d273341a7ad9 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Sun, 15 Nov 2020 15:46:33 +0900 Subject: [PATCH 1/7] Use plot$mapping in compute_geom_2() --- R/layer.r | 8 ++++++-- R/plot-build.r | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/R/layer.r b/R/layer.r index 0db859978a..16b0699352 100644 --- a/R/layer.r +++ b/R/layer.r @@ -353,11 +353,15 @@ Layer <- ggproto("Layer", NULL, self$position$compute_layer(data, params, layout) }, - compute_geom_2 = function(self, data) { + compute_geom_2 = function(self, data, plot) { # Combine aesthetics, defaults, & params if (empty(data)) return(data) - aesthetics <- self$mapping + if (self$inherit.aes) { + aesthetics <- defaults(self$mapping, plot$mapping) + } else { + aesthetics <- self$mapping + } modifiers <- aesthetics[is_scaled_aes(aesthetics) | is_staged_aes(aesthetics)] self$geom$use_defaults(data, self$aes_params, modifiers) diff --git a/R/plot-build.r b/R/plot-build.r index dcac060141..139bc45fcd 100644 --- a/R/plot-build.r +++ b/R/plot-build.r @@ -97,7 +97,7 @@ ggplot_build.ggplot <- function(plot) { } # Fill in defaults etc. - data <- by_layer(function(l, d) l$compute_geom_2(d)) + data <- by_layer(function(l, d) l$compute_geom_2(d, plot)) # Let layer stat have a final say before rendering data <- by_layer(function(l, d) l$finish_statistics(d)) From b6de158a763130dce29c8619b79e7d9ff1757b49 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Sun, 15 Nov 2020 15:50:05 +0900 Subject: [PATCH 2/7] Use default_mapping in guide_geom() --- R/guide-legend.r | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/R/guide-legend.r b/R/guide-legend.r index 6aa593342c..2ac5ed09d4 100644 --- a/R/guide-legend.r +++ b/R/guide-legend.r @@ -261,7 +261,11 @@ guide_geom.legend <- function(guide, layers, default_mapping) { n <- vapply(layer$aes_params, length, integer(1)) params <- layer$aes_params[n == 1] - aesthetics <- layer$mapping + if (layer$inherit.aes) { + aesthetics <- defaults(layer$mapping, default_mapping) + } else { + aesthetics <- layer$mapping + } modifiers <- aesthetics[is_scaled_aes(aesthetics) | is_staged_aes(aesthetics)] data <- tryCatch( From 08de90d4454a6f6c27f102b7ea281e94390c4e64 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 16 Nov 2020 23:50:48 +0900 Subject: [PATCH 3/7] Merge mapping in setup_layer() --- R/layer.r | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/R/layer.r b/R/layer.r index 16b0699352..2d78703f57 100644 --- a/R/layer.r +++ b/R/layer.r @@ -203,6 +203,12 @@ Layer <- ggproto("Layer", NULL, # hook to allow a layer access to the final layer data # in input form and to global plot info setup_layer = function(self, data, plot) { + if (isTRUE(self$inherit.aes)) { + self$mapping <- defaults(self$mapping, plot$mapping) + # defaults() strips class, but it needs to be preserved for now + class(self$mapping) <- "uneval" + } + data }, From 301edfd09833eec08f9f853f2d19afb64e58361c Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 17 Nov 2020 00:00:32 +0900 Subject: [PATCH 4/7] Stop merging a mapping repeatedly --- R/guide-legend.r | 6 +----- R/layer.r | 16 ++-------------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/R/guide-legend.r b/R/guide-legend.r index 2ac5ed09d4..6aa593342c 100644 --- a/R/guide-legend.r +++ b/R/guide-legend.r @@ -261,11 +261,7 @@ guide_geom.legend <- function(guide, layers, default_mapping) { n <- vapply(layer$aes_params, length, integer(1)) params <- layer$aes_params[n == 1] - if (layer$inherit.aes) { - aesthetics <- defaults(layer$mapping, default_mapping) - } else { - aesthetics <- layer$mapping - } + aesthetics <- layer$mapping modifiers <- aesthetics[is_scaled_aes(aesthetics) | is_staged_aes(aesthetics)] data <- tryCatch( diff --git a/R/layer.r b/R/layer.r index 2d78703f57..c4e7a67ad8 100644 --- a/R/layer.r +++ b/R/layer.r @@ -213,12 +213,7 @@ Layer <- ggproto("Layer", NULL, }, compute_aesthetics = function(self, data, plot) { - # For annotation geoms, it is useful to be able to ignore the default aes - if (self$inherit.aes) { - aesthetics <- defaults(self$mapping, plot$mapping) - } else { - aesthetics <- self$mapping - } + aesthetics <- self$mapping # Drop aesthetics that are set or calculated set <- names(aesthetics) %in% names(self$aes_params) @@ -295,9 +290,6 @@ Layer <- ggproto("Layer", NULL, # Assemble aesthetics from layer, plot and stat mappings aesthetics <- self$mapping - if (self$inherit.aes) { - aesthetics <- defaults(aesthetics, plot$mapping) - } aesthetics <- defaults(aesthetics, self$stat$default_aes) aesthetics <- compact(aesthetics) @@ -363,11 +355,7 @@ Layer <- ggproto("Layer", NULL, # Combine aesthetics, defaults, & params if (empty(data)) return(data) - if (self$inherit.aes) { - aesthetics <- defaults(self$mapping, plot$mapping) - } else { - aesthetics <- self$mapping - } + aesthetics <- self$mapping modifiers <- aesthetics[is_scaled_aes(aesthetics) | is_staged_aes(aesthetics)] self$geom$use_defaults(data, self$aes_params, modifiers) From 857560647f2ee012778a34b30e167fc336642c1f Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 17 Nov 2020 00:17:15 +0900 Subject: [PATCH 5/7] Revert compute_geom_2() --- R/layer.r | 2 +- R/plot-build.r | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/layer.r b/R/layer.r index c4e7a67ad8..79a8ec6c99 100644 --- a/R/layer.r +++ b/R/layer.r @@ -351,7 +351,7 @@ Layer <- ggproto("Layer", NULL, self$position$compute_layer(data, params, layout) }, - compute_geom_2 = function(self, data, plot) { + compute_geom_2 = function(self, data) { # Combine aesthetics, defaults, & params if (empty(data)) return(data) diff --git a/R/plot-build.r b/R/plot-build.r index 139bc45fcd..dcac060141 100644 --- a/R/plot-build.r +++ b/R/plot-build.r @@ -97,7 +97,7 @@ ggplot_build.ggplot <- function(plot) { } # Fill in defaults etc. - data <- by_layer(function(l, d) l$compute_geom_2(d, plot)) + data <- by_layer(function(l, d) l$compute_geom_2(d)) # Let layer stat have a final say before rendering data <- by_layer(function(l, d) l$finish_statistics(d)) From f90ce6859217d818629cf3ff0ce9c0f964624dd1 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 17 Nov 2020 00:18:53 +0900 Subject: [PATCH 6/7] Migrate a comment --- R/layer.r | 1 + 1 file changed, 1 insertion(+) diff --git a/R/layer.r b/R/layer.r index 79a8ec6c99..44c75fc0df 100644 --- a/R/layer.r +++ b/R/layer.r @@ -203,6 +203,7 @@ Layer <- ggproto("Layer", NULL, # hook to allow a layer access to the final layer data # in input form and to global plot info setup_layer = function(self, data, plot) { + # For annotation geoms, it is useful to be able to ignore the default aes if (isTRUE(self$inherit.aes)) { self$mapping <- defaults(self$mapping, plot$mapping) # defaults() strips class, but it needs to be preserved for now From 0e9b5fc97c517d3cefe59cd9e1746ee01810f9d4 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Sun, 6 Dec 2020 17:16:55 +0900 Subject: [PATCH 7/7] Add a NEWS item --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index edb832d2c5..768b14c81a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -24,6 +24,9 @@ * ggplot2 now requires R >= 3.3 (#4247). +* Fix a bug that `after_stat()` and `after_scale()` cannot refer to aesthetics + if it's specified in the plot-global mapping (@yutannihilation, #4260). + # ggplot2 3.3.2 This is a small release focusing on fixing regressions introduced in 3.3.1.