WIP: Register default scales via themes

[alanocallaghan]

Resolve #2691

Add default.scales component of themes.

[clauswilke]

We don't normally use argument names ending in _, as in theme_. Does theme = theme() not work here?

[alanocallaghan]

I got promise already under evaluation, otherwise I'd have gone with that. I can do theme=ggplot2::theme() instead. See reprex:

x <- "Hello world"
f <- function(get = get("x")) {
  print(get)
}
f()
#> Error in print(get): promise already under evaluation: recursive default argument reference or earlier problems?

[clauswilke]

As I commented elsewhere, just write theme = NULL.

[clauswilke]

I think theme should have a default here. I think theme = NULL makes sense.

[clauswilke]

This should be commented, just like the other parts of the code in this function.

[clauswilke]

It's probably best to add a specific element class here. PR #2749 adds element_geom(), so maybe this one here could be called element_scales(). But I'd like to have input from @yutannihilation or @thomasp85 here.

[clauswilke]

Also, if we add a specific element class then we check then and there that the element values are functions and then we can avoid the mode() check in find_global() (to be moved into find_scale()).

[clauswilke]

Apologies, my picture is evolving as I'm thinking about this: I don't think you should modify find_global(). That makes no sense. Instead, move the theme lookup code into find_scale(). If find_scale() doesn't find an appropriate scale in the theme, then it can try the global lookup with the unmodified find_global(). Makes much more sense.

[alanocallaghan]

Thanks, yeah I figured find_global was the wrong place entirely. find_scale makes sense, there's no reason to look in the theme in general when doing lookup.

[clauswilke]

@alanocallaghan I realized there's a different issue and it suggests that this PR may be premature. We don't have a good way of feeding additional parameters into an existing scale, such as breaks, break labels, or expansion parameters. Without this mechanism, setting a default scale in a theme has limited utility and can actually cause confusion, when people want to set the breaks and cannot do this while also retaining the color scheme their theme defines. Issue #3962 is related and may have to be resolved first.

@baptiste It seems to me that you're proposing an entirely new way of defining scales. Your proposal may make sense, but I'd like to see a proof-of-concept implementation to really understand what exactly the proposal is and how it'd interact with all the currently existing ggplot2 code. I'd also want to make sure that it works for shape, size, alpha, etc. scales in addition to color scales.

[alanocallaghan]

Alright no worries, this isn't a particularly urgent thing for me - just something I had noticed and fancied playing with. Let me know if you get a handle on how that might progress - happy to lend a hand there too if needed. Otherwise ping this when it's ready to progress.