From dd9c09de18fa8919f18466c61e6d0e28d6eb17ab Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 23 Nov 2023 00:39:33 +0100 Subject: [PATCH 01/13] add double-dispatch for layers --- NAMESPACE | 1 + R/plot-construction.R | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/NAMESPACE b/NAMESPACE index c5f5a94219..ccd9f7a6b8 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -57,6 +57,7 @@ S3method(ggplot_add,Coord) S3method(ggplot_add,Facet) S3method(ggplot_add,Guides) S3method(ggplot_add,Layer) +S3method(ggplot_add,Layer.default) S3method(ggplot_add,Scale) S3method(ggplot_add,by) S3method(ggplot_add,data.frame) diff --git a/R/plot-construction.R b/R/plot-construction.R index c4cafd2dc8..028282dc29 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -165,7 +165,13 @@ ggplot_add.by <- function(object, plot, object_name) { } #' @export +#' @method ggplot_add Layer ggplot_add.Layer <- function(object, plot, object_name) { + UseMethod("ggplot_add.Layer", plot) +} + +#' @export +ggplot_add.Layer.default <- function(object, plot, object_name) { plot$layers <- append(plot$layers, object) # Add any new labels From aaefb1eca29caa11405e51ba0c105f0bc55b714e Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Sun, 26 Nov 2023 11:23:03 +0100 Subject: [PATCH 02/13] Convert `ggplot_add()` to S7 generic --- DESCRIPTION | 1 + NAMESPACE | 22 +++----- R/ggplot2-package.R | 1 + R/plot-construction.R | 115 ++++++++++++++++++++++++++++-------------- 4 files changed, 86 insertions(+), 53 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 77cb1ce98d..3d44dff8ec 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -40,6 +40,7 @@ Imports: MASS, mgcv, rlang (>= 1.1.0), + S7, scales (>= 1.2.0), stats, tibble, diff --git a/NAMESPACE b/NAMESPACE index ccd9f7a6b8..439ffdda8b 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -51,21 +51,6 @@ S3method(get_alt_text,ggplot_built) S3method(get_alt_text,gtable) S3method(ggplot,"function") S3method(ggplot,default) -S3method(ggplot_add,"NULL") -S3method(ggplot_add,"function") -S3method(ggplot_add,Coord) -S3method(ggplot_add,Facet) -S3method(ggplot_add,Guides) -S3method(ggplot_add,Layer) -S3method(ggplot_add,Layer.default) -S3method(ggplot_add,Scale) -S3method(ggplot_add,by) -S3method(ggplot_add,data.frame) -S3method(ggplot_add,default) -S3method(ggplot_add,labels) -S3method(ggplot_add,list) -S3method(ggplot_add,theme) -S3method(ggplot_add,uneval) S3method(ggplot_build,ggplot) S3method(ggplot_gtable,ggplot_built) S3method(grid.draw,absoluteGrob) @@ -701,6 +686,13 @@ import(gtable) import(rlang) import(scales) import(vctrs) +importFrom(S7,"method<-") +importFrom(S7,S7_dispatch) +importFrom(S7,class_any) +importFrom(S7,class_list) +importFrom(S7,method) +importFrom(S7,new_S3_class) +importFrom(S7,new_generic) importFrom(glue,glue) importFrom(glue,glue_collapse) importFrom(lifecycle,deprecated) diff --git a/R/ggplot2-package.R b/R/ggplot2-package.R index 539f03db97..35e656b725 100644 --- a/R/ggplot2-package.R +++ b/R/ggplot2-package.R @@ -5,6 +5,7 @@ #' @import scales grid gtable rlang vctrs #' @importFrom glue glue glue_collapse #' @importFrom lifecycle deprecated +#' @importFrom S7 class_any class_list method method<- new_generic new_S3_class S7_dispatch #' @importFrom stats setNames #' @importFrom utils head tail ## usethis namespace: end diff --git a/R/plot-construction.R b/R/plot-construction.R index 028282dc29..49dc21c64d 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -87,49 +87,80 @@ add_ggplot <- function(p, object, objectname) { #' #' @keywords internal #' @export -ggplot_add <- function(object, plot, object_name) { - UseMethod("ggplot_add") -} -#' @export -ggplot_add.default <- function(object, plot, object_name) { +ggplot_add <- new_generic( + "ggplot_add", + dispatch_args = c("object", "plot"), + fun = function(object, plot, object_name) S7_dispatch() +) + +class_ggplot <- new_S3_class("ggplot") + +method( + ggplot_add, + list(object = class_any, plot = class_ggplot) +) <- function(object, plot, object_name) { cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") } -#' @export -ggplot_add.NULL <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("NULL"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot } -#' @export -ggplot_add.data.frame <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("data.frame"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot$data <- object plot } -#' @export -ggplot_add.function <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("function"), plot = class_ggplot) +) <- function(object, plot, object_name) { cli::cli_abort(c( - "Can't add {.var {object_name}} to a {.cls ggplot} object", + "Can't add {.var {object_name}} to a {.cls ggplot} object", "i" = "Did you forget to add parentheses, as in {.fn {object_name}}?" )) } -#' @export -ggplot_add.theme <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("theme"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot$theme <- add_theme(plot$theme, object) plot } -#' @export -ggplot_add.Scale <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("Scale"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot$scales$add(object) plot } -#' @export -ggplot_add.labels <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("labels"), plot = class_ggplot) +) <- function(object, plot, object_name) { update_labels(plot, object) } -#' @export -ggplot_add.Guides <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("Guides"), plot = class_ggplot) +) <- function(object, plot, object_name) { update_guides(plot, object) } -#' @export -ggplot_add.uneval <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("uneval"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot$mapping <- defaults(object, plot$mapping) # defaults() doesn't copy class, so copy it. class(plot$mapping) <- class(object) @@ -138,8 +169,11 @@ ggplot_add.uneval <- function(object, plot, object_name) { names(labels) <- names(object) update_labels(plot, labels) } -#' @export -ggplot_add.Coord <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("Coord"), plot = class_ggplot) +) <- function(object, plot, object_name) { if (!isTRUE(plot$coordinates$default)) { cli::cli_inform("Coordinate system already present. Adding new coordinate system, which will replace the existing one.") } @@ -147,31 +181,36 @@ ggplot_add.Coord <- function(object, plot, object_name) { plot$coordinates <- object plot } -#' @export -ggplot_add.Facet <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = new_S3_class("Facet"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot$facet <- object plot } -#' @export -ggplot_add.list <- function(object, plot, object_name) { + +method( + ggplot_add, + list(object = class_list, plot = class_ggplot) +) <- function(object, plot, object_name) { for (o in object) { plot <- plot %+% o } plot } -#' @export -ggplot_add.by <- function(object, plot, object_name) { - ggplot_add.list(object, plot, object_name) -} -#' @export -#' @method ggplot_add Layer -ggplot_add.Layer <- function(object, plot, object_name) { - UseMethod("ggplot_add.Layer", plot) +method( + ggplot_add, + list(object = new_S3_class("by"), plot = class_ggplot) +) <- function(object, plot, object_name) { + ggplot_add.list(object, plot, object_name) } -#' @export -ggplot_add.Layer.default <- function(object, plot, object_name) { +method( + ggplot_add, + list(object = new_S3_class("Layer"), plot = class_ggplot) +) <- function(object, plot, object_name) { plot$layers <- append(plot$layers, object) # Add any new labels From cd7cc4ed54510e17701dc8a3df5574e2257ed38e Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 11:02:03 +0100 Subject: [PATCH 03/13] Declare classes beforehand --- R/plot-construction.R | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/R/plot-construction.R b/R/plot-construction.R index 49dc21c64d..4d40f2f174 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -102,6 +102,19 @@ method( cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") } +# Class declarations for S7 dispatch. If S7 gets implemented more broadly, +# consider moving these to a new file. +class_ggplot <- S7::new_S3_class("ggplot") +class_theme <- S7::new_S3_class("theme") +class_scale <- S7::new_S3_class("Scale") +class_labels <- S7::new_S3_class("labels") +class_guides <- S7::new_S3_class("Guides") +class_aes <- S7::new_S3_class("uneval") +class_coord <- S7::new_S3_class("Coord") +class_facet <- S7::new_S3_class("Facet") +class_by <- S7::new_S3_class("by") +class_layer <- S7::new_S3_class("Layer") + method( ggplot_add, list(object = new_S3_class("NULL"), plot = class_ggplot) From a4e4c35b2d26b2d861b6603d5dbfb6de2d7f6c67 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 11:02:41 +0100 Subject: [PATCH 04/13] Follow Hadley's advice --- NAMESPACE | 7 -- R/ggplot2-package.R | 1 - R/plot-construction.R | 222 ++++++++++++++++++------------------------ 3 files changed, 96 insertions(+), 134 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index 439ffdda8b..b24d627889 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -686,13 +686,6 @@ import(gtable) import(rlang) import(scales) import(vctrs) -importFrom(S7,"method<-") -importFrom(S7,S7_dispatch) -importFrom(S7,class_any) -importFrom(S7,class_list) -importFrom(S7,method) -importFrom(S7,new_S3_class) -importFrom(S7,new_generic) importFrom(glue,glue) importFrom(glue,glue_collapse) importFrom(lifecycle,deprecated) diff --git a/R/ggplot2-package.R b/R/ggplot2-package.R index 35e656b725..539f03db97 100644 --- a/R/ggplot2-package.R +++ b/R/ggplot2-package.R @@ -5,7 +5,6 @@ #' @import scales grid gtable rlang vctrs #' @importFrom glue glue glue_collapse #' @importFrom lifecycle deprecated -#' @importFrom S7 class_any class_list method method<- new_generic new_S3_class S7_dispatch #' @importFrom stats setNames #' @importFrom utils head tail ## usethis namespace: end diff --git a/R/plot-construction.R b/R/plot-construction.R index 4d40f2f174..785820488d 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -70,7 +70,7 @@ add_ggplot <- function(p, object, objectname) { if (is.null(object)) return(p) p <- plot_clone(p) - p <- ggplot_add(object, p, objectname) + p <- ggplot_add(object, p, object_name = objectname) set_last_plot(p) p } @@ -87,20 +87,7 @@ add_ggplot <- function(p, object, objectname) { #' #' @keywords internal #' @export -ggplot_add <- new_generic( - "ggplot_add", - dispatch_args = c("object", "plot"), - fun = function(object, plot, object_name) S7_dispatch() -) - -class_ggplot <- new_S3_class("ggplot") - -method( - ggplot_add, - list(object = class_any, plot = class_ggplot) -) <- function(object, plot, object_name) { - cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") -} +ggplot_add <- S7::new_generic("ggplot_add", c("object", "plot")) # Class declarations for S7 dispatch. If S7 gets implemented more broadly, # consider moving these to a new file. @@ -115,129 +102,112 @@ class_facet <- S7::new_S3_class("Facet") class_by <- S7::new_S3_class("by") class_layer <- S7::new_S3_class("Layer") -method( - ggplot_add, - list(object = new_S3_class("NULL"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot -} +S7::method(ggplot_add, list(S7::class_any, class_ggplot)) <- + function(object, plot, object_name) { + cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") + } -method( - ggplot_add, - list(object = new_S3_class("data.frame"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot$data <- object - plot -} +# Cannot currently double dispatch on NULL directly +# replace `S7::new_S3_class("NULL")` with `NULL` when S7 version > 0.1.1 +S7::method(ggplot_add, list(S7::new_S3_class("NULL"), class_ggplot)) <- + function(object, plot, object_name) { + plot + } -method( - ggplot_add, - list(object = new_S3_class("function"), plot = class_ggplot) -) <- function(object, plot, object_name) { - cli::cli_abort(c( - "Can't add {.var {object_name}} to a {.cls ggplot} object", - "i" = "Did you forget to add parentheses, as in {.fn {object_name}}?" - )) -} +S7::method(ggplot_add, list(S7::class_data.frame, class_ggplot)) <- + function(object, plot, object_name) { + plot$data <- object + plot + } -method( - ggplot_add, - list(object = new_S3_class("theme"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot$theme <- add_theme(plot$theme, object) - plot -} +S7::method(ggplot_add, list(S7::class_function, class_ggplot)) <- + function(object, plot, object_name) { + cli::cli_abort(c( + "Can't add {.var {object_name}} to a {.cls ggplot} object", + "i" = "Did you forget to add parentheses, as in {.fn {object_name}}?" + )) + } -method( - ggplot_add, - list(object = new_S3_class("Scale"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot$scales$add(object) - plot -} +S7::method(ggplot_add, list(class_theme, class_ggplot)) <- + function(object, plot, object_name) { + plot$theme <- add_theme(plot$theme, object) + plot + } -method( - ggplot_add, - list(object = new_S3_class("labels"), plot = class_ggplot) -) <- function(object, plot, object_name) { - update_labels(plot, object) -} +S7::method(ggplot_add, list(class_scale, class_ggplot)) <- + function(object, plot, object_name) { + plot$scales$add(object) + plot + } -method( - ggplot_add, - list(object = new_S3_class("Guides"), plot = class_ggplot) -) <- function(object, plot, object_name) { - update_guides(plot, object) -} +S7::method(ggplot_add, list(class_labels, class_ggplot)) <- + function(object, plot, object_name) { + update_labels(plot, object) + } -method( - ggplot_add, - list(object = new_S3_class("uneval"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot$mapping <- defaults(object, plot$mapping) - # defaults() doesn't copy class, so copy it. - class(plot$mapping) <- class(object) - - labels <- make_labels(object) - names(labels) <- names(object) - update_labels(plot, labels) -} +S7::method(ggplot_add, list(class_guides, class_ggplot)) <- + function(object, plot, object_name) { + update_guides(plot, object) + } -method( - ggplot_add, - list(object = new_S3_class("Coord"), plot = class_ggplot) -) <- function(object, plot, object_name) { - if (!isTRUE(plot$coordinates$default)) { - cli::cli_inform("Coordinate system already present. Adding new coordinate system, which will replace the existing one.") +S7::method(ggplot_add, list(class_aes, class_ggplot)) <- + function(object, plot, object_name) { + plot$mapping <- defaults(object, plot$mapping) + # defaults() doesn't copy class, so copy it. + class(plot$mapping) <- class(object) + + labels <- make_labels(object) + names(labels) <- names(object) + update_labels(plot, labels) } - plot$coordinates <- object - plot -} +S7::method(ggplot_add, list(class_coord, class_ggplot)) <- + function(object, plot, object_name) { + if (!isTRUE(plot$coordinates$default)) { + cli::cli_inform("Coordinate system already present. Adding new coordinate system, which will replace the existing one.") + } -method( - ggplot_add, - list(object = new_S3_class("Facet"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot$facet <- object - plot -} + plot$coordinates <- object + plot + } -method( - ggplot_add, - list(object = class_list, plot = class_ggplot) -) <- function(object, plot, object_name) { - for (o in object) { - plot <- plot %+% o +S7::method(ggplot_add, list(class_facet, class_ggplot)) <- + function(object, plot, object_name) { + plot$facet <- object + plot } - plot -} -method( - ggplot_add, - list(object = new_S3_class("by"), plot = class_ggplot) -) <- function(object, plot, object_name) { - ggplot_add.list(object, plot, object_name) -} +S7::method(ggplot_add, list(S7::class_list, class_ggplot)) <- + function(object, plot, object_name) { + for (o in object) { + plot <- plot %+% o + } + plot + } -method( - ggplot_add, - list(object = new_S3_class("Layer"), plot = class_ggplot) -) <- function(object, plot, object_name) { - plot$layers <- append(plot$layers, object) - - # Add any new labels - mapping <- make_labels(object$mapping) - default <- lapply(make_labels(object$stat$default_aes), function(l) { - attr(l, "fallback") <- TRUE - l - }) - new_labels <- defaults(mapping, default) - current_labels <- plot$labels - current_fallbacks <- vapply(current_labels, function(l) isTRUE(attr(l, "fallback")), logical(1)) - plot$labels <- defaults(current_labels[!current_fallbacks], new_labels) - if (any(current_fallbacks)) { - plot$labels <- defaults(plot$labels, current_labels) - } - plot -} +S7::method(ggplot_add, list(class_by, class_ggplot)) <- + function(object, plot, object_name) { + S7::method(ggplot_add, list(class_list, class_ggplot))( + object, plot, object_name + ) + } + +S7::method(ggplot_add, list(class_layer, class_ggplot)) <- + function(object, plot, object_name) { + plot$layers <- append(plot$layers, object) + + # Add any new labels + mapping <- make_labels(object$mapping) + default <- lapply(make_labels(object$stat$default_aes), function(l) { + attr(l, "fallback") <- TRUE + l + }) + new_labels <- defaults(mapping, default) + current_labels <- plot$labels + current_fallbacks <- vapply(current_labels, function(l) isTRUE(attr(l, "fallback")), logical(1)) + plot$labels <- defaults(current_labels[!current_fallbacks], new_labels) + if (any(current_fallbacks)) { + plot$labels <- defaults(plot$labels, current_labels) + } + plot + } From 1b00a60affbe162aa0acb3b21d076e51085ad173 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 11:08:22 +0100 Subject: [PATCH 05/13] redocument --- R/plot-construction.R | 4 +++- man/ggplot_add.Rd | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/R/plot-construction.R b/R/plot-construction.R index 785820488d..6b05af01c2 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -81,7 +81,9 @@ add_ggplot <- function(p, object, objectname) { #' #' @param object An object to add to the plot #' @param plot The ggplot object to add `object` to -#' @param object_name The name of the object to add +#' @param ... Additional arguments to pass to the methods. Typically, an +#' `object_name` argument that gives a display name for `object` to use +#' in error messages. #' #' @return A modified ggplot object #' diff --git a/man/ggplot_add.Rd b/man/ggplot_add.Rd index 0bd2e2a698..3986ef2962 100644 --- a/man/ggplot_add.Rd +++ b/man/ggplot_add.Rd @@ -4,14 +4,16 @@ \alias{ggplot_add} \title{Add custom objects to ggplot} \usage{ -ggplot_add(object, plot, object_name) +ggplot_add(object, plot, ...) } \arguments{ \item{object}{An object to add to the plot} \item{plot}{The ggplot object to add \code{object} to} -\item{object_name}{The name of the object to add} +\item{...}{Additional arguments to pass to the methods. Typically, an +\code{object_name} argument that gives a display name for \code{object} to use +in error messages.} } \value{ A modified ggplot object From d8bf5e3b1099aa81064ca5a597a60bd8829a55de Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 16:48:54 +0100 Subject: [PATCH 06/13] class declaration to front --- R/all-classes.R | 11 +++++++++++ R/plot-construction.R | 13 ------------- 2 files changed, 11 insertions(+), 13 deletions(-) create mode 100644 R/all-classes.R diff --git a/R/all-classes.R b/R/all-classes.R new file mode 100644 index 0000000000..e92f68b185 --- /dev/null +++ b/R/all-classes.R @@ -0,0 +1,11 @@ +# Class declarations for S7 dispatch. +class_theme <- S7::new_S3_class("theme") +class_scale <- S7::new_S3_class("Scale") +class_labels <- S7::new_S3_class("labels") +class_guides <- S7::new_S3_class("Guides") +class_aes <- S7::new_S3_class("uneval") +class_coord <- S7::new_S3_class("Coord") +class_facet <- S7::new_S3_class("Facet") +class_by <- S7::new_S3_class("by") +class_layer <- S7::new_S3_class("Layer") +class_scales_list <- S7::new_S3_class("ScalesList") diff --git a/R/plot-construction.R b/R/plot-construction.R index 6b05af01c2..ec8060d88f 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -91,19 +91,6 @@ add_ggplot <- function(p, object, objectname) { #' @export ggplot_add <- S7::new_generic("ggplot_add", c("object", "plot")) -# Class declarations for S7 dispatch. If S7 gets implemented more broadly, -# consider moving these to a new file. -class_ggplot <- S7::new_S3_class("ggplot") -class_theme <- S7::new_S3_class("theme") -class_scale <- S7::new_S3_class("Scale") -class_labels <- S7::new_S3_class("labels") -class_guides <- S7::new_S3_class("Guides") -class_aes <- S7::new_S3_class("uneval") -class_coord <- S7::new_S3_class("Coord") -class_facet <- S7::new_S3_class("Facet") -class_by <- S7::new_S3_class("by") -class_layer <- S7::new_S3_class("Layer") - S7::method(ggplot_add, list(S7::class_any, class_ggplot)) <- function(object, plot, object_name) { cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") From ffc347edd2c43b0e85f2c90a281ebca86c5ff809 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 16:49:38 +0100 Subject: [PATCH 07/13] ggplot as S7 class --- R/plot.R | 86 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 36 deletions(-) diff --git a/R/plot.R b/R/plot.R index 4494b774bc..794dc49eb4 100644 --- a/R/plot.R +++ b/R/plot.R @@ -1,3 +1,6 @@ + +gg <- S7::new_class("gg", abstract = TRUE) + #' Create a new ggplot #' #' `ggplot()` initializes a ggplot object. It can be used to @@ -101,49 +104,60 @@ #' mapping = aes(x = group, y = group_mean), data = group_means_df, #' colour = 'red', size = 3 #' ) -ggplot <- function(data = NULL, mapping = aes(), ..., - environment = parent.frame()) { - UseMethod("ggplot") -} +ggplot <- S7::new_class( + name = "ggplot", parent = gg, + properties = list( + data = S7::class_any, + layers = S7::class_list, + scales = class_scales_list, + guides = class_guides, + mapping = class_aes, + theme = class_theme, + coordinates = class_coord, + facet = class_facet, + labels = S7::class_list, + plot_env = S7::class_environment + ), + constructor = function(data = NULL, mapping = aes(), ..., + environment = parent.frame()) { -#' @export -ggplot.default <- function(data = NULL, mapping = aes(), ..., - environment = parent.frame()) { - if (!missing(mapping) && !inherits(mapping, "uneval")) { - cli::cli_abort(c( - "{.arg mapping} should be created with {.fn aes}.", - "x" = "You've supplied a {.cls {class(mapping)[1]}} object" - )) - } + if (!missing(mapping) && !inherits(mapping, "uneval")) { + cli::cli_abort(c( + "{.arg mapping} should be created with {.fn aes}.", + "x" = "You've supplied a {.cls {class(mapping)[1]}} object." + )) + } - data <- fortify(data, ...) + if (is.function(data)) { + cli::cli_abort(c( + "{.arg data} cannot be a function.", + "i" = "Have you misspelled the {.arg data} argument in {.fn ggplot}?" + )) + } - p <- structure(list( - data = data, - layers = list(), - scales = scales_list(), - guides = guides_list(), - mapping = mapping, - theme = list(), - coordinates = coord_cartesian(default = TRUE), - facet = facet_null(), - plot_env = environment - ), class = c("gg", "ggplot")) + data <- fortify(data, ...) - p$labels <- make_labels(mapping) + obj <- S7::new_object( + S7::S7_object(), + data = data, + layers = list(), + scales = scales_list(), + guides = guides_list(), + mapping = mapping, + theme = theme(), + coordinates = coord_cartesian(default = TRUE), + facet = facet_null(), + labels = make_labels(mapping), + plot_env = environment + ) + + set_last_plot(obj) + obj + } +) - set_last_plot(p) - p } -#' @export -ggplot.function <- function(data = NULL, mapping = aes(), ..., - environment = parent.frame()) { - # Added to avoid functions end in ggplot.default - cli::cli_abort(c( - "{.arg data} cannot be a function.", - "i" = "Have you misspelled the {.arg data} argument in {.fn ggplot}" - )) } plot_clone <- function(plot) { From 92f1e000f56ee10cfbf3f7c68c53966595f75a91 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 16:50:19 +0100 Subject: [PATCH 08/13] adapt methods --- R/plot-construction.R | 39 ++++++++++++++++++++++----------------- R/plot.R | 7 +++++++ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/R/plot-construction.R b/R/plot-construction.R index ec8060d88f..2648dcde68 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -1,3 +1,6 @@ +#' @include plot.R +NULL + #' Add components to a plot #' #' `+` is the key to constructing sophisticated ggplot2 graphics. It @@ -91,25 +94,25 @@ add_ggplot <- function(p, object, objectname) { #' @export ggplot_add <- S7::new_generic("ggplot_add", c("object", "plot")) -S7::method(ggplot_add, list(S7::class_any, class_ggplot)) <- +S7::method(ggplot_add, list(S7::class_any, ggplot)) <- function(object, plot, object_name) { cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") } # Cannot currently double dispatch on NULL directly # replace `S7::new_S3_class("NULL")` with `NULL` when S7 version > 0.1.1 -S7::method(ggplot_add, list(S7::new_S3_class("NULL"), class_ggplot)) <- +S7::method(ggplot_add, list(S7::new_S3_class("NULL"), ggplot)) <- function(object, plot, object_name) { plot } -S7::method(ggplot_add, list(S7::class_data.frame, class_ggplot)) <- +S7::method(ggplot_add, list(S7::class_data.frame, ggplot)) <- function(object, plot, object_name) { plot$data <- object plot } -S7::method(ggplot_add, list(S7::class_function, class_ggplot)) <- +S7::method(ggplot_add, list(S7::class_function, ggplot)) <- function(object, plot, object_name) { cli::cli_abort(c( "Can't add {.var {object_name}} to a {.cls ggplot} object", @@ -117,40 +120,42 @@ S7::method(ggplot_add, list(S7::class_function, class_ggplot)) <- )) } -S7::method(ggplot_add, list(class_theme, class_ggplot)) <- +S7::method(ggplot_add, list(class_theme, ggplot)) <- function(object, plot, object_name) { plot$theme <- add_theme(plot$theme, object) plot } -S7::method(ggplot_add, list(class_scale, class_ggplot)) <- +S7::method(ggplot_add, list(class_scale, ggplot)) <- function(object, plot, object_name) { plot$scales$add(object) plot } -S7::method(ggplot_add, list(class_labels, class_ggplot)) <- +S7::method(ggplot_add, list(class_labels, ggplot)) <- function(object, plot, object_name) { update_labels(plot, object) } -S7::method(ggplot_add, list(class_guides, class_ggplot)) <- +S7::method(ggplot_add, list(class_guides, ggplot)) <- function(object, plot, object_name) { update_guides(plot, object) } -S7::method(ggplot_add, list(class_aes, class_ggplot)) <- +S7::method(ggplot_add, list(class_aes, ggplot)) <- function(object, plot, object_name) { - plot$mapping <- defaults(object, plot$mapping) + mapping <- defaults(object, plot$mapping) # defaults() doesn't copy class, so copy it. - class(plot$mapping) <- class(object) + class(mapping) <- class(object) + S7::prop(plot, "mapping") <- mapping + labels <- make_labels(object) names(labels) <- names(object) update_labels(plot, labels) } -S7::method(ggplot_add, list(class_coord, class_ggplot)) <- +S7::method(ggplot_add, list(class_coord, ggplot)) <- function(object, plot, object_name) { if (!isTRUE(plot$coordinates$default)) { cli::cli_inform("Coordinate system already present. Adding new coordinate system, which will replace the existing one.") @@ -160,13 +165,13 @@ S7::method(ggplot_add, list(class_coord, class_ggplot)) <- plot } -S7::method(ggplot_add, list(class_facet, class_ggplot)) <- +S7::method(ggplot_add, list(class_facet, ggplot)) <- function(object, plot, object_name) { plot$facet <- object plot } -S7::method(ggplot_add, list(S7::class_list, class_ggplot)) <- +S7::method(ggplot_add, list(S7::class_list, ggplot)) <- function(object, plot, object_name) { for (o in object) { plot <- plot %+% o @@ -174,14 +179,14 @@ S7::method(ggplot_add, list(S7::class_list, class_ggplot)) <- plot } -S7::method(ggplot_add, list(class_by, class_ggplot)) <- +S7::method(ggplot_add, list(class_by, ggplot)) <- function(object, plot, object_name) { - S7::method(ggplot_add, list(class_list, class_ggplot))( + S7::method(ggplot_add, list(class_list, ggplot))( object, plot, object_name ) } -S7::method(ggplot_add, list(class_layer, class_ggplot)) <- +S7::method(ggplot_add, list(class_layer, ggplot)) <- function(object, plot, object_name) { plot$layers <- append(plot$layers, object) diff --git a/R/plot.R b/R/plot.R index 794dc49eb4..5876772c4b 100644 --- a/R/plot.R +++ b/R/plot.R @@ -156,8 +156,15 @@ ggplot <- S7::new_class( } ) +S7::method(`$`, ggplot) <- function(x, i) { + if (!S7::prop_exists(x, i)) { + return(NULL) + } + S7::prop(x, i) } +S7::method(`$<-`, ggplot) <- function(x, ...) { + S7::`prop<-`(x, ...) } plot_clone <- function(plot) { From 33f242b69d91a4c4cf502c8f9d0c488b114e5954 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 16:51:04 +0100 Subject: [PATCH 09/13] roxygenate --- DESCRIPTION | 3 ++- NAMESPACE | 2 -- tests/testthat/_snaps/plot.md | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 3d44dff8ec..e90be52897 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -91,6 +91,7 @@ Collate: 'compat-plyr.R' 'utilities.R' 'aes.R' + 'all-classes.R' 'utilities-checks.R' 'legend-draw.R' 'geom-.R' @@ -197,9 +198,9 @@ Collate: 'margins.R' 'performance.R' 'plot-build.R' + 'plot.R' 'plot-construction.R' 'plot-last.R' - 'plot.R' 'position-.R' 'position-collide.R' 'position-dodge.R' diff --git a/NAMESPACE b/NAMESPACE index b24d627889..c3a6b4c81e 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -49,8 +49,6 @@ S3method(fortify,tbl_df) S3method(get_alt_text,ggplot) S3method(get_alt_text,ggplot_built) S3method(get_alt_text,gtable) -S3method(ggplot,"function") -S3method(ggplot,default) S3method(ggplot_build,ggplot) S3method(ggplot_gtable,ggplot_built) S3method(grid.draw,absoluteGrob) diff --git a/tests/testthat/_snaps/plot.md b/tests/testthat/_snaps/plot.md index 6dd7cfd427..85882c7ccd 100644 --- a/tests/testthat/_snaps/plot.md +++ b/tests/testthat/_snaps/plot.md @@ -1,12 +1,12 @@ # ggplot() throws informative errors `mapping` should be created with `aes()`. - x You've supplied a object + x You've supplied a object. --- `data` cannot be a function. - i Have you misspelled the `data` argument in `ggplot()` + i Have you misspelled the `data` argument in `ggplot()`? # construction have user friendly errors From 1fc5bea097bbefcb35233fc0e066bec0f94b0abe Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 27 Nov 2023 16:57:15 +0100 Subject: [PATCH 10/13] add method registration --- R/zzz.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/R/zzz.R b/R/zzz.R index 0dcfd407cf..3158922541 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -30,6 +30,10 @@ on_load( vars <- dplyr::vars } ) +on_load( + S7::methods_register() +) + .onLoad <- function(...) { run_on_load() } From 75b860c9d808920d28e2b2269025cf829c50ceb2 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 28 Nov 2023 10:21:03 +0100 Subject: [PATCH 11/13] Bring back classic `ggplot()` --- NAMESPACE | 3 ++ R/plot-construction.R | 28 +++++++------- R/plot.R | 89 ++++++++++++++++++++++++------------------- 3 files changed, 66 insertions(+), 54 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index c3a6b4c81e..7dd13e633a 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -49,6 +49,8 @@ S3method(fortify,tbl_df) S3method(get_alt_text,ggplot) S3method(get_alt_text,ggplot_built) S3method(get_alt_text,gtable) +S3method(ggplot,"function") +S3method(ggplot,default) S3method(ggplot_build,ggplot) S3method(ggplot_gtable,ggplot_built) S3method(grid.draw,absoluteGrob) @@ -212,6 +214,7 @@ export(PositionJitter) export(PositionJitterdodge) export(PositionNudge) export(PositionStack) +export(S7_ggplot) export(Scale) export(ScaleBinned) export(ScaleBinnedPosition) diff --git a/R/plot-construction.R b/R/plot-construction.R index 2648dcde68..6b04a20502 100644 --- a/R/plot-construction.R +++ b/R/plot-construction.R @@ -94,25 +94,25 @@ add_ggplot <- function(p, object, objectname) { #' @export ggplot_add <- S7::new_generic("ggplot_add", c("object", "plot")) -S7::method(ggplot_add, list(S7::class_any, ggplot)) <- +S7::method(ggplot_add, list(S7::class_any, S7_ggplot)) <- function(object, plot, object_name) { cli::cli_abort("Can't add {.var {object_name}} to a {.cls ggplot} object.") } # Cannot currently double dispatch on NULL directly # replace `S7::new_S3_class("NULL")` with `NULL` when S7 version > 0.1.1 -S7::method(ggplot_add, list(S7::new_S3_class("NULL"), ggplot)) <- +S7::method(ggplot_add, list(S7::new_S3_class("NULL"), S7_ggplot)) <- function(object, plot, object_name) { plot } -S7::method(ggplot_add, list(S7::class_data.frame, ggplot)) <- +S7::method(ggplot_add, list(S7::class_data.frame, S7_ggplot)) <- function(object, plot, object_name) { plot$data <- object plot } -S7::method(ggplot_add, list(S7::class_function, ggplot)) <- +S7::method(ggplot_add, list(S7::class_function, S7_ggplot)) <- function(object, plot, object_name) { cli::cli_abort(c( "Can't add {.var {object_name}} to a {.cls ggplot} object", @@ -120,29 +120,29 @@ S7::method(ggplot_add, list(S7::class_function, ggplot)) <- )) } -S7::method(ggplot_add, list(class_theme, ggplot)) <- +S7::method(ggplot_add, list(class_theme, S7_ggplot)) <- function(object, plot, object_name) { plot$theme <- add_theme(plot$theme, object) plot } -S7::method(ggplot_add, list(class_scale, ggplot)) <- +S7::method(ggplot_add, list(class_scale, S7_ggplot)) <- function(object, plot, object_name) { plot$scales$add(object) plot } -S7::method(ggplot_add, list(class_labels, ggplot)) <- +S7::method(ggplot_add, list(class_labels, S7_ggplot)) <- function(object, plot, object_name) { update_labels(plot, object) } -S7::method(ggplot_add, list(class_guides, ggplot)) <- +S7::method(ggplot_add, list(class_guides, S7_ggplot)) <- function(object, plot, object_name) { update_guides(plot, object) } -S7::method(ggplot_add, list(class_aes, ggplot)) <- +S7::method(ggplot_add, list(class_aes, S7_ggplot)) <- function(object, plot, object_name) { mapping <- defaults(object, plot$mapping) # defaults() doesn't copy class, so copy it. @@ -155,7 +155,7 @@ S7::method(ggplot_add, list(class_aes, ggplot)) <- update_labels(plot, labels) } -S7::method(ggplot_add, list(class_coord, ggplot)) <- +S7::method(ggplot_add, list(class_coord, S7_ggplot)) <- function(object, plot, object_name) { if (!isTRUE(plot$coordinates$default)) { cli::cli_inform("Coordinate system already present. Adding new coordinate system, which will replace the existing one.") @@ -165,13 +165,13 @@ S7::method(ggplot_add, list(class_coord, ggplot)) <- plot } -S7::method(ggplot_add, list(class_facet, ggplot)) <- +S7::method(ggplot_add, list(class_facet, S7_ggplot)) <- function(object, plot, object_name) { plot$facet <- object plot } -S7::method(ggplot_add, list(S7::class_list, ggplot)) <- +S7::method(ggplot_add, list(S7::class_list, S7_ggplot)) <- function(object, plot, object_name) { for (o in object) { plot <- plot %+% o @@ -179,14 +179,14 @@ S7::method(ggplot_add, list(S7::class_list, ggplot)) <- plot } -S7::method(ggplot_add, list(class_by, ggplot)) <- +S7::method(ggplot_add, list(class_by, S7_ggplot)) <- function(object, plot, object_name) { S7::method(ggplot_add, list(class_list, ggplot))( object, plot, object_name ) } -S7::method(ggplot_add, list(class_layer, ggplot)) <- +S7::method(ggplot_add, list(class_layer, S7_ggplot)) <- function(object, plot, object_name) { plot$layers <- append(plot$layers, object) diff --git a/R/plot.R b/R/plot.R index 5876772c4b..3d50ada468 100644 --- a/R/plot.R +++ b/R/plot.R @@ -104,7 +104,52 @@ gg <- S7::new_class("gg", abstract = TRUE) #' mapping = aes(x = group, y = group_mean), data = group_means_df, #' colour = 'red', size = 3 #' ) -ggplot <- S7::new_class( +ggplot <- function(data = NULL, mapping = aes(), ..., + environment = parent.frame()) { + UseMethod("ggplot") +} + +#' @export +ggplot.default <- function(data = NULL, mapping = aes(), ..., + environment = parent.frame()) { + if (!missing(mapping) && !inherits(mapping, "uneval")) { + cli::cli_abort(c( + "{.arg mapping} should be created with {.fn aes}.", + "x" = "You've supplied a {.cls {class(mapping)[1]}} object." + )) + } + + data <- fortify(data, ...) + + p <- S7_ggplot( + data = data, + layers = list(), + scales = scales_list(), + guides = guides_list(), + mapping = mapping, + theme = theme(), + coordinates = coord_cartesian(default = TRUE), + facet = facet_null(), + labels = make_labels(mapping), + plot_env = environment + ) + + set_last_plot(p) + p +} + +#' @export +ggplot.function <- function(data = NULL, mapping = aes(), ..., + environment = parent.frame()) { + cli::cli_abort(c( + "{.arg data} cannot be a function.", + "i" = "Have you misspelled the {.arg data} argument in {.fn ggplot}?" + )) +} + +#' @export +#' @rdname ggplot +S7_ggplot <- S7::new_class( name = "ggplot", parent = gg, properties = list( data = S7::class_any, @@ -117,53 +162,17 @@ ggplot <- S7::new_class( facet = class_facet, labels = S7::class_list, plot_env = S7::class_environment - ), - constructor = function(data = NULL, mapping = aes(), ..., - environment = parent.frame()) { - - if (!missing(mapping) && !inherits(mapping, "uneval")) { - cli::cli_abort(c( - "{.arg mapping} should be created with {.fn aes}.", - "x" = "You've supplied a {.cls {class(mapping)[1]}} object." - )) - } - - if (is.function(data)) { - cli::cli_abort(c( - "{.arg data} cannot be a function.", - "i" = "Have you misspelled the {.arg data} argument in {.fn ggplot}?" - )) - } - - data <- fortify(data, ...) - - obj <- S7::new_object( - S7::S7_object(), - data = data, - layers = list(), - scales = scales_list(), - guides = guides_list(), - mapping = mapping, - theme = theme(), - coordinates = coord_cartesian(default = TRUE), - facet = facet_null(), - labels = make_labels(mapping), - plot_env = environment - ) - - set_last_plot(obj) - obj - } + ) ) -S7::method(`$`, ggplot) <- function(x, i) { +S7::method(`$`, S7_ggplot) <- function(x, i) { if (!S7::prop_exists(x, i)) { return(NULL) } S7::prop(x, i) } -S7::method(`$<-`, ggplot) <- function(x, ...) { +S7::method(`$<-`, S7_ggplot) <- function(x, ...) { S7::`prop<-`(x, ...) } From 7e7a0b1a6c64c5d07bd54fe7961a26adbd5cad19 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 28 Nov 2023 10:23:19 +0100 Subject: [PATCH 12/13] deal with bug --- R/plot.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/plot.R b/R/plot.R index 3d50ada468..6df6b50437 100644 --- a/R/plot.R +++ b/R/plot.R @@ -176,6 +176,9 @@ S7::method(`$<-`, S7_ggplot) <- function(x, ...) { S7::`prop<-`(x, ...) } +# Deal with S7 bug: https://github.com/RConsortium/S7/issues/390 +rm(`$`, `$<-`) + plot_clone <- function(plot) { p <- plot p$scales <- plot$scales$clone() From 30edf644c04edd9afd444617367a056c3ccbfb61 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 28 Nov 2023 10:45:08 +0100 Subject: [PATCH 13/13] Document S7_ggplot class separately --- R/plot.R | 31 +++++++++++++++++++++++++- man/S7_ggplot.Rd | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 man/S7_ggplot.Rd diff --git a/R/plot.R b/R/plot.R index 6df6b50437..818a34ca70 100644 --- a/R/plot.R +++ b/R/plot.R @@ -147,8 +147,37 @@ ggplot.function <- function(data = NULL, mapping = aes(), ..., )) } +#' ggplot class +#' +#' The ggplot class is implemented using S7 and has properties needed to +#' build and render a plot. +#' +#' @param data Any object that can be used with [`fortify()`] to yield a +#' ``. +#' @param layers A `` containing `` objects. Typically +#' an empty `` that will be filled when adding layers using `+`. +#' @param scales A `` ggproto object that manages scales that +#' are added to the plot. +#' @param guides A `` ggproto object that manages guides that are +#' added to the plot. +#' @param mapping An `` object constructed with [`aes()`] containing +#' the default aesthetic mappings. +#' @param theme A `` object constructed with [`theme()`] containing +#' non-data visual settings. +#' @param coordinates A `` ggproto object that manages the interpretation +#' of position aesthetics. +#' @param facet A `` ggproto object that manages the display of data +#' subsets. +#' @param labels A named `` of `character`s and `expression`s giving +#' aesthetic-label pairs. +#' @param plot_env An `` in which the plot was created. +#' +#' @details +#' The purpose of the ggplot class object is to allow developers to extend +#' their own versions of a ggplot class. Users should instead use the +#' [`ggplot()`] interface to construct a new plot. +#' #' @export -#' @rdname ggplot S7_ggplot <- S7::new_class( name = "ggplot", parent = gg, properties = list( diff --git a/man/S7_ggplot.Rd b/man/S7_ggplot.Rd new file mode 100644 index 0000000000..f413d55ce8 --- /dev/null +++ b/man/S7_ggplot.Rd @@ -0,0 +1,58 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/plot.R +\name{S7_ggplot} +\alias{S7_ggplot} +\title{ggplot class} +\usage{ +S7_ggplot( + data = class_missing, + layers = class_missing, + scales = class_missing, + guides = class_missing, + mapping = class_missing, + theme = class_missing, + coordinates = class_missing, + facet = class_missing, + labels = class_missing, + plot_env = class_missing +) +} +\arguments{ +\item{data}{Any object that can be used with \code{\link[=fortify]{fortify()}} to yield a +\verb{}.} + +\item{layers}{A \verb{} containing \verb{} objects. Typically +an empty \verb{} that will be filled when adding layers using \code{+}.} + +\item{scales}{A \verb{} ggproto object that manages scales that +are added to the plot.} + +\item{guides}{A \verb{} ggproto object that manages guides that are +added to the plot.} + +\item{mapping}{An \verb{} object constructed with \code{\link[=aes]{aes()}} containing +the default aesthetic mappings.} + +\item{theme}{A \verb{} object constructed with \code{\link[=theme]{theme()}} containing +non-data visual settings.} + +\item{coordinates}{A \verb{} ggproto object that manages the interpretation +of position aesthetics.} + +\item{facet}{A \verb{} ggproto object that manages the display of data +subsets.} + +\item{labels}{A named \verb{} of \code{character}s and \code{expression}s giving +aesthetic-label pairs.} + +\item{plot_env}{An \verb{} in which the plot was created.} +} +\description{ +The ggplot class is implemented using S7 and has properties needed to +build and render a plot. +} +\details{ +The purpose of the ggplot class object is to allow developers to extend +their own versions of a ggplot class. Users should instead use the +\code{\link[=ggplot]{ggplot()}} interface to construct a new plot. +}