-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Register default scales via themes #2691
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
Comments
This function would also have to be modified: Lines 110 to 121 in bcc3ae5
It is similarly called from |
Just for completeness I'll re-iterate my personal preference for a revised scale_colour|fill_xx interface.
where besides the distinction discrete/continuous I would argue that the essential difference is the palette. An alternative description might be a generic The interface might require a new kind of object to cater for all cases – a string to refer to built-in palettes, a function (potentially a functor) e.g for user-defined gradient/gradient2/gradientn and their parameters (midpoint, NA, etc.). To me this feels more natural, and more in line with other design choices such as The current array of special-case functions could still coexist, similarly to |
I would love to see an implementation of this to see how feasible it is. |
@cpsievert Would you be interested in trying to take this on? I don't have the capacity right now to tackle this. |
@clauswilke I have other priorities at the moment, too. I don't think this should necessary block #3833, though. If there's some reason why it should, let me know, and I'll try and find time to look into this. |
Agreed. Just think about whether your proposed API is right if we assume at some point scales can be set from the theme. This may or may not be the case. I haven't thought it through carefully. |
The theme is by default an empty list when I've not checked in detail but it seems the complete theme is built by Line 444 in 660aad2
when it's called in ggplot_gtable.ggplot_built Line 166 in 660aad2
I would assume you could build the theme during ggplot2/tests/testthat/test-theme.r Lines 219 to 227 in 660aad2
), but these are just checking that the built plot doesn't have a complete theme. That doesn't seem like a major issue, since the theme is completed when being drawn in any case. Of course it's quite probable I'm overlooking something (the tests are presumably there for a reason...). |
Quick implementation here https://github.com/Alanocallaghan/ggplot2/tree/2691-default-scales-via-themes As I mentioned in previous comment, I moved For the purposes of a visual example I modified the reprex: devtools::load_all("~/Documents/github/ggplot2")
ggplot(mtcars, aes(mpg, cyl, color=factor(cyl))) + geom_point() + theme_grey()
|
Thanks for looking into this. I've got a few requests that will help us move forward.
library(ggplot2)
p <- ggplot(mtcars, aes(disp, mpg)) + geom_point()
p theme_set(theme_bw())
p Created on 2020-04-29 by the reprex package (v0.3.0) |
Oh, and for the line that you consider "silly" please add a comment to your code indicating what is happening and why you think it is silly. This will prevent us from accidentally forgetting that there may have been a problem that needed addressing. |
Sure. Can you clarify point 3? I don't follow. |
Let's say ggplot(iris, aes(Sepal.Length, Sepal.Width, color = Species)) +
geom_point() +
theme_A() +
theme(
default.scales = ... # what goes here?
) Or, I like the color and fill scales, but I also want to define a shape scale. How do I do that? ggplot(iris, aes(Sepal.Length, Sepal.Width, color = Species, shape = Species)) +
geom_point() +
theme_A() +
theme(
default.scales = ... # what goes here?
) |
Partial specification of the default scales runs into an undefined library("ggplot2")
library("viridis")
theme_A <- function() {
theme(
default.scales = c(
"scale_colour_continuous" = scale_colour_viridis
),
complete = TRUE
)
}
ggplot(mtcars, aes(disp, mpg, color=cyl, fill = factor(cyl))) +
geom_boxplot() +
theme_A() +
theme(
default.scales = c(
scale_fill_discrete = scale_fill_brewer
)
)
#> Error: No method for merging list into list Trace is:
Implementing a very basic library("ggplot2")
library("viridis")
#> Loading required package: viridisLite
## Without changing the default scale
ggplot(mtcars, aes(mpg, cyl, color=factor(cyl))) + geom_point() theme_A <- function() {
theme(
default.scales = c(
"scale_colour_continuous" = scale_colour_viridis
),
complete = TRUE
)
}
## With a theme that sets one scale (colour) and not another (fill)
ggplot(mtcars, aes(disp, mpg, fill = factor(cyl))) +
geom_boxplot() +
geom_jitter(aes(color=mpg)) +
theme_A() ## use the same theme that sets colour and also set a default for fill
ggplot(mtcars, aes(disp, mpg, fill = factor(cyl))) +
geom_boxplot() +
geom_jitter(aes(color=mpg)) +
theme_A() +
theme(
default.scales = c(
scale_fill_discrete = scale_fill_brewer
)
) p <- ggplot(mtcars, aes(disp, mpg, color=mpg)) + geom_point()
# printing it before setting a theme means it has the default scales
p # but if we set theme it has the updated scales
theme_set(theme_A())
p |
Not sure I like the interface where we put all the scales into one element, like so: theme(
default.scales = element_scales(
scale_fill_discrete = scale_fill_viridis_d,
scale_fill_continuous = scale_fill_viridis_c
)
) It seems cumbersome. What about adding elements for the different types of scales, like so: theme(
scales.discrete = element_scales(
fill = scale_fill_viridis_d
),
scales.continuous = element_scales(
fill = scale_fill_viridis_c
)
) @hadley @yutannihilation @thomasp85 Any thoughts on a possible interface? |
I vote for
More seriously though, what do people think of my remarks above regarding the explosion of names for fill and colour scales? The gist of it is,
The reason I bring this up is that in my view specifying a palette in a theme seems much more straightforward than a scale, especially if they are all a bit different. I do realise that not all colour scales work with just a palette, there are also gradient, gradientn, etc. but I suspect a common interface (or rather, a handful, like viridis_c vs viridis_d) abstracting away the specific colours, would make for a more robust and tidy interface. Specifying a palette is also something that would integrate more easily with other workflows, e.g external CSS values, json specs, etc. |
@baptiste I think your point is orthogonal to this issue. Regardless of whether we introduce scale functions that allow us to pick scales by palette names, we still have to set appropriate scale functions. And keep in mind that in addition to colors, there are also scales to set shape, size, alpha, linetype, etc. I think an interface that allows us to pick an arbitrary function is ultimately needed, even if there are simplified approaches for special cases. For the colorspace package, I attempted to implement generic scale functions that take a And, if you want to define your own palettes, you'll quickly run into the situation where you have to specify at least 9 numeric parameters, and still you're limited in the type of color gradients you can generate: There's also the paletteer package that attempts to provide a unified interface to many different color scales: |
@baptiste Also, my proposed interface could be generalized easily to handle both scale functions and palette names, by simply accepting character strings that are interpreted as palettes: theme(
scales.discrete = element_scales(
fill = "viridis" # creates discrete color scale using "viridis" palette
),
scales.continuous = element_scales(
fill = "viridis" # creates continuous color scale using "viridis" palette
)
) |
I don't think it has to be orthogonal – my point, in relation to this specific thread, is that it'd be more convenient to define some information about the scales in the theme, but not necessarily the scale object itself. In its bare-bones implementation it would mean passing just a vector of colours, but obviously that's not enough for all types of colour scales. A list of parameters, however, e.g Does that make more sense? |
(to push the analogy, the geom-defaults-in-theme branch doesn't define geom_point in the theme; it defines its default appearance) |
I agree fwiw. Arguably it's not a huge problem because presumably default scales will be set once. With the latter more explicit approach it's probably easier to validate what's being set. For example as baptiste pointed out you may not want people setting a default for
One problem I could see with this approach is that you may have to essentially duplicate the scale class to hold sufficient information to adequately configure default scales. |
I'm considering this fixed by #5946. It sets the palettes via the theme rather than the complete scale, but other scale properties like limits or breaks are data-dependent decisions that shouldn't be hardcoded into the theme anyway, as themes mostly comprise non-data decisions. |
As previously mentioned in the discussion of #2239: It would be nice if default scales could be set via themes.
Currently, default scales are defined as functions that are named
scale_
aesthetic
_
data-type
()
, whereaesthetic
is the name of the aesthetic anddata-type
represents the data type, i.e., continuous, discrete, date, etc. For example,scale_colour_discrete()
is the default scale function for discrete colors.The definition of these defaults is distributed all over the scales code, because for some scales the default scale is the only scale that exists (example:
ggplot2/R/scale-alpha.r
Line 31 in 77fc5c7
ggplot2/R/zxx.r
Line 6 in 77fc5c7
It would be nice if there were a way to change the defaults other than to change the global function definitions, and in particular if the defaults could be linked to the theme. For example, the theme could have an entry
default_scales = list("scale_color_discrete" = scale_color_hue, "scale_color_continuous" = scale_color_gradient, ...)
that lists the default scales to be used with this theme.In terms of implementation, I see two problems, and it's not clear to me how much of a hurdle they might be. First, the code that searches for the default scales currently doesn't have access to the theme:
ggplot2/R/scale-type.R
Lines 1 to 20 in bcc3ae5
It is called from this function:
ggplot2/R/scales-.r
Line 91 in bcc3ae5
which is called from here:
ggplot2/R/layer.r
Line 214 in a7b3135
and here:
ggplot2/R/layer.r
Line 274 in a7b3135
Second, it's not immediately clear to me whether we even have the complete theme by the time these functions are called. They are ultimately called in
plot_build()
here:ggplot2/R/plot-build.r
Line 48 in 3f63d62
If by that time the entire theme is assembled, then I assume it could be handed down through all these functions to the point where the scale is looked up. Otherwise, this proposal would be infeasible.
The text was updated successfully, but these errors were encountered: