tidyverse / ggplot2

An implementation of the Grammar of Graphics in R
https://ggplot2.tidyverse.org
Other
6.55k stars 2.03k forks source link

Support scale functions in packages not attached via scale_type() #4705

Open krlmlr opened 2 years ago

krlmlr commented 2 years ago

This is from https://github.com/r-lib/pillar/pull/404 that implements scale_type.pillar_num() (returning "num") and scale_x_num() and scale_y_num() . The scales become available only after the pillar package is attached, see below. This seems to be due to how scale lookup is performed in https://github.com/tidyverse/ggplot2/blob/c89c265a57fd71f8a0288ce81037296aadc0a012/R/scale-type.R#L13.

Should we allow scale_type() to return a reference to an environment where to (also) look for scale functions? Perhaps if our scale_type() returns "pillar::num", or as an attribute to the value returned by scale_type() ?

data <- tibble::tibble(
  x = pillar::num((1:10) / 100, fixed_exponent = -3, notation = "eng"),
  y = pillar::num((1:10) / 100, scale = 100, label = "%"),
  z = pillar::num(10^(-5:4), notation = "si")
)
data
#> # A tibble: 10 × 3
#>         x     y     z
#>     <eng>     %  <si>
#>  1  10e-3     1   10µ
#>  2  20e-3     2  100µ
#>  3  30e-3     3    1m
#>  4  40e-3     4   10m
#>  5  50e-3     5  100m
#>  6  60e-3     6    1 
#>  7  70e-3     7   10 
#>  8  80e-3     8  100 
#>  9  90e-3     9    1k
#> 10 100e-3    10   10k

ggplot2::ggplot(data, ggplot2::aes(x, y)) +
  ggplot2::geom_point()

library(pillar)

ggplot2::ggplot(data, ggplot2::aes(x, y)) +
  ggplot2::geom_point()

Created on 2021-12-28 by the reprex package (v2.0.1)

https://github.com/r-quantities/units/pull/294 is also affected, CC @Enchufa2.

Enchufa2 commented 2 years ago

+1. There's less trouble with geoms and other kinds of functions because you need to call them explicitly. Scales, however, most of the time work automagically, so it would be great to have a way to tell ggplot2 how to find them independently of whether the user attaches third-party packages or not. This could be done via some dynamic registration mechanism (similar to dynamic registration of S3 methods), but the proposal by @krlmlr, via scale_type, would be most convenient for us.

hadley commented 2 years ago

@thomasp85 do you think we could get this in the next minor release?

thomasp85 commented 2 years ago

sure

hadley commented 2 years ago

Alternatively, if no scale was found in an attached package we could look through loadedNamespaces()?

In either case, I thinking seeing a concrete implementation in a PR would be useful.

TimTaylor commented 1 year ago

+1. I'm running in to similar problems which I've been trying to debug. Just commenting to note #4799 fixed things for me.