-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Allow default geom aesthetics to be set from theme #2749
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
Changes from 6 commits
19b0cc7
7dc76d5
21acff9
0bc4041
ecf1356
42acbd3
9e1305f
58a677d
23fdf39
4bc838c
af8c462
a4f5c2f
bf34c2b
ff4c7ec
1445665
0630120
dc2daa0
ce57ba9
844df2a
517f056
ddad370
5f312e8
08feee1
c3f8387
33e7dc6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,11 +107,18 @@ Geom <- ggproto("Geom", | |
setup_data = function(data, params) data, | ||
|
||
# Combine data with defaults and set aesthetics from parameters | ||
use_defaults = function(self, data, params = list()) { | ||
use_defaults = function(self, data, params = list(), theme) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the code related to evaluating aesthetics in the special theme environment should be pulled out into its own function so that we can think about it in isolation, and then test it. |
||
|
||
# evaluates defaults given plot theme | ||
if (length(theme) == 0) theme <- theme_grey() | ||
dpseidel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
env <- new.env() | ||
env$theme <- theme | ||
defaults <- rlang::eval_tidy(self$default_aes, env) | ||
|
||
# Fill in missing aesthetics with their defaults | ||
missing_aes <- setdiff(names(self$default_aes), names(data)) | ||
missing_aes <- setdiff(names(defaults), names(data)) | ||
|
||
missing_eval <- lapply(self$default_aes[missing_aes], rlang::eval_tidy) | ||
missing_eval <- lapply(defaults[missing_aes], rlang::eval_tidy) | ||
# Needed for geoms with defaults set to NULL (e.g. GeomSf) | ||
missing_eval <- compact(missing_eval) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,11 @@ | |
#' @rdname update_defaults | ||
update_geom_defaults <- function(geom, new) { | ||
g <- check_subclass(geom, "Geom", env = parent.frame()) | ||
old <- g$default_aes | ||
|
||
env <- new.env() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If geoms are themeable, I think we can deprecate this function, so we should just leave as is and in the documentation mark it as internal, and point people towards the new theme system. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we deprecate this, we will presumably need to make all aesthetics themeable so as to not lose the functionality? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah. In that case we should pull this out into a function that can be shared with |
||
env$theme <- theme_get() | ||
old <- rlang::eval_tidy(g$default_aes, env) | ||
|
||
g$default_aes <- defaults(rename_aes(new), old) | ||
invisible() | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.