Skip to content

POC: Alternative way of determining parameters #6101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions R/geom-.R
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ Geom <- ggproto("Geom",

default_aes = aes(),

default_params = NULL,

Comment on lines +66 to +67
Copy link
Collaborator Author

@teunbrand teunbrand Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The Geom and Stat classes have a new field that can hold a named list of default parameters.

draw_key = draw_key_point,

handle_na = function(self, data, params) {
Expand All @@ -79,7 +81,7 @@ Geom <- ggproto("Geom",
}

# Trim off extra parameters
params <- params[intersect(names(params), self$parameters())]
params <- filter_args(params, self$draw_panel)
Copy link
Collaborator Author

@teunbrand teunbrand Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We then filter the parameters on the go in the compute/draw_layer() and compute/draw_panel() methods. Implementation of filter_args() in the utils file.


if (nlevels(as.factor(data$PANEL)) > 1L) {
data_panels <- split(data, data$PANEL)
Expand All @@ -96,8 +98,9 @@ Geom <- ggproto("Geom",

draw_panel = function(self, data, panel_params, coord, ...) {
groups <- split(data, factor(data$group))
params <- filter_args(list2(...), self$draw_group)
grobs <- lapply(groups, function(group) {
self$draw_group(group, panel_params, coord, ...)
inject(self$draw_group(group, panel_params = panel_params, coord = coord, !!!params))
})

ggname(snake_class(self), gTree(
Expand Down Expand Up @@ -208,6 +211,9 @@ Geom <- ggproto("Geom",
extra_params = c("na.rm"),

parameters = function(self, extra = FALSE) {
if (!is.null(self$default_params)) {
return(names(self$default_params))
}
Comment on lines +214 to +216
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We simply return the name of the default parameter list for checking layer() input. For backward compatibility, if this list isn't present, we resort to the old way of determining input.

# Look first in draw_panel. If it contains ... then look in draw groups
panel_args <- names(ggproto_formals(self$draw_panel))
group_args <- names(ggproto_formals(self$draw_group))
Expand Down
8 changes: 7 additions & 1 deletion R/geom-boxplot.R
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,13 @@ GeomBoxplot <- ggproto("GeomBoxplot", Geom,

# need to declare `width` here in case this geom is used with a stat that
# doesn't have a `width` parameter (e.g., `stat_identity`).
extra_params = c("na.rm", "width", "orientation", "outliers"),
default_params = list(
na.rm = FALSE, width = NULL, orientation = NA, outliers = TRUE,
lineend = "butt", linejoin = "mitre", fatten = 2, outlier.colour = NULL,
outlier.fill = NULL, outlier.shape = NULL, outlier.size = NULL,
outlier.stroke = 0.5, outlier.alpha = NULL, notch = FALSE, notchwidth = 0.5,
staplewidth = 0, varwidth = FALSE, flipped_aes = FALSE
),
Comment on lines +181 to +187
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. As a geom example, here is how we could implement the default parameter list for GeomBoxplot


setup_params = function(data, params) {
params$flipped_aes <- has_flipped_aes(data, params)
Expand Down
7 changes: 4 additions & 3 deletions R/geom-violin.R
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ GeomViolin <- ggproto("GeomViolin", Geom,
# Needed for coord_polar and such
newdata <- vec_rbind0(newdata, newdata[1,])
newdata <- flip_data(newdata, flipped_aes)
params <- filter_args(list(...), GeomPolygon$draw_panel)

# Draw quantiles if requested, so long as there is non-zero y range
if (length(draw_quantiles) > 0 & !scales::zero_range(range(data$y))) {
Expand All @@ -183,15 +184,15 @@ GeomViolin <- ggproto("GeomViolin", Geom,
quantile_grob <- if (nrow(both) == 0) {
zeroGrob()
} else {
GeomPath$draw_panel(both, ...)
inject(GeomPath$draw_panel(both, !!!params))
}

ggname("geom_violin", grobTree(
GeomPolygon$draw_panel(newdata, ...),
inject(GeomPolygon$draw_panel(newdata, !!!params)),
Copy link
Collaborator Author

@teunbrand teunbrand Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Here is a downside of the method: we cannot simply pass dots here as GeomViolin recieves more parameters than GeomPolygon needs. This was the only Geom where this was a problem though.

quantile_grob)
)
} else {
ggname("geom_violin", GeomPolygon$draw_panel(newdata, ...))
ggname("geom_violin", inject(GeomPolygon$draw_panel(newdata, !!!params)))
}
},

Expand Down
11 changes: 6 additions & 5 deletions R/layer.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@
#' `NA`, the default, includes if any aesthetics are mapped.
#' `FALSE` never includes, and `TRUE` always includes.
#' It can also be a named logical vector to finely select the aesthetics to
#' display. To include legend keys for all levels, even
#' when no data exists, use `TRUE`. If `NA`, all levels are shown in legend,
#' display. To include legend keys for all levels, even
#' when no data exists, use `TRUE`. If `NA`, all levels are shown in legend,
#' but unobserved levels are omitted.
#' @param inherit.aes If `FALSE`, overrides the default aesthetics,
#' rather than combining with them. This is most useful for helper functions
Expand Down Expand Up @@ -360,8 +360,8 @@ Layer <- ggproto("Layer", NULL,
compute_statistic = function(self, data, layout) {
if (empty(data))
return(data_frame0())

self$computed_stat_params <- self$stat$setup_params(data, self$stat_params)
params <- defaults(self$stat_params, self$stat$default_params)
self$computed_stat_params <- self$stat$setup_params(data, params)
data <- self$stat$setup_data(data, self$computed_stat_params)
self$stat$compute_layer(data, self$computed_stat_params, layout)
},
Expand Down Expand Up @@ -430,7 +430,8 @@ Layer <- ggproto("Layer", NULL,
c(names(data), names(self$aes_params)),
snake_class(self$geom)
)
self$computed_geom_params <- self$geom$setup_params(data, c(self$geom_params, self$aes_params))
params <- defaults(c(self$geom_params, self$aes_params), self$geom$default_params)
self$computed_geom_params <- self$geom$setup_params(data, params)
Comment on lines +433 to +434
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Before we add computed components to the parameters, we initialise the parameters from the new field

self$geom$setup_data(data, self$computed_geom_params)
},

Expand Down
13 changes: 11 additions & 2 deletions R/stat-.R
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Stat <- ggproto("Stat",

optional_aes = character(),

default_params = NULL,

Comment on lines +76 to +77
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We take the same approach with Stats

setup_params = function(data, params) {
params
},
Expand Down Expand Up @@ -102,7 +104,7 @@ Stat <- ggproto("Stat",
)

# Trim off extra parameters
params <- params[intersect(names(params), self$parameters())]
params <- filter_args(params, self$compute_panel)

args <- c(list(data = quote(data), scales = quote(scales)), params)
dapply(data, "PANEL", function(data) {
Expand All @@ -121,8 +123,11 @@ Stat <- ggproto("Stat",
if (empty(data)) return(data_frame0())

groups <- split(data, data$group)

params <- filter_args(list2(...), self$compute_group)

stats <- lapply(groups, function(group) {
self$compute_group(data = group, scales = scales, ...)
inject(self$compute_group(data = group, scales = scales, !!!params))
})

# Record columns that are not constant within groups. We will drop them later.
Expand Down Expand Up @@ -194,6 +199,10 @@ Stat <- ggproto("Stat",
# See discussion at Geom$parameters()
extra_params = "na.rm",
parameters = function(self, extra = FALSE) {
if (!is.null(self$default_params)) {
return(names(self$default_params))
}

# Look first in compute_panel. If it contains ... then look in compute_group
panel_args <- names(ggproto_formals(self$compute_panel))
group_args <- names(ggproto_formals(self$compute_group))
Expand Down
6 changes: 5 additions & 1 deletion R/stat-summary.R
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ stat_summary <- function(mapping = NULL, data = NULL,
StatSummary <- ggproto("StatSummary", Stat,
required_aes = c("x", "y"),

extra_params = c("na.rm", "orientation", "fun.data", "fun.max", "fun.min", "fun.args"),
default_params = list(
na.rm = FALSE, orientation = NA,
fun.data = NULL, fun = NULL, fun.max = NULL, fun.min = NULL,
fun.args = list()
),
Comment on lines +184 to +188
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. As an example stat, here is how we could implement the default parameters for StatSummary


setup_params = function(data, params) {
params$flipped_aes <- has_flipped_aes(data, params)
Expand Down
13 changes: 13 additions & 0 deletions R/utilities.R
Original file line number Diff line number Diff line change
Expand Up @@ -901,3 +901,16 @@
utils::install.packages(pkg)
is_installed(pkg)
}

filter_args <- function(args, fun) {
fmls <- if (inherits(fun, "ggproto_method")) {
names(ggproto_formals(fun))
} else {
names(formals(fun))

Check warning on line 909 in R/utilities.R

View check run for this annotation

Codecov / codecov/patch

R/utilities.R#L909

Added line #L909 was not covered by tests
}
if ("..." %in% fmls) {
return(args)
}
args[intersect(names(args), fmls)]
}

Loading