-
Notifications
You must be signed in to change notification settings - Fork 2.1k
POC: Alternative way of determining parameters#6101
POC: Alternative way of determining parameters#6101teunbrand wants to merge 7 commits intotidyverse:mainfrom
Conversation
This PR explores to fix #1516.
It follows the approach proposed in #1516 (comment).
I've numbered the comments in the code below so it'd be easier to follow.
This PR is currently only implemented for 1 stat and 2 geoms, complying with 'a minimal version' of the approach.
It is for this reason still a POC, and we still have to decide if we like this approach.
| default_params = NULL, | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The Geom and Stat classes have a new field that can hold a named list of default parameters.
| if (!is.null(self$default_params)) { | ||
| return(names(self$default_params)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.
| params <- defaults(c(self$geom_params, self$aes_params), self$geom$default_params) | ||
| self$computed_geom_params <- self$geom$setup_params(data, params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Before we add computed components to the parameters, we initialise the parameters from the new field
|
|
||
| # Trim off extra parameters | ||
| params <- params[intersect(names(params), self$parameters())] | ||
| params <- filter_args(params, self$draw_panel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We then filter the parameters on the go in the
compute/draw_layer()andcompute/draw_panel()methods. Implementation offilter_args()in the utils file.
| default_params = NULL, | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We take the same approach with Stats
| default_params = list( | ||
| na.rm = FALSE, orientation = NA, | ||
| fun.data = NULL, fun = NULL, fun.max = NULL, fun.min = NULL, | ||
| fun.args = list() | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- As an example stat, here is how we could implement the default parameters for
StatSummary
| 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 | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- As a geom example, here is how we could implement the default parameter list for
GeomBoxplot
|
|
||
| ggname("geom_violin", grobTree( | ||
| GeomPolygon$draw_panel(newdata, ...), | ||
| inject(GeomPolygon$draw_panel(newdata, !!!params)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.