ggplot2 icon indicating copy to clipboard operation
ggplot2 copied to clipboard

Allow scale name to be a function.

Open eliocamp opened this issue 3 years ago • 13 comments

I think it might be useful to have the possibility of passing a function to scale names like so:

library(ggplot2)

df <- expand.grid(X1 = 1:10, X2 = 1:10)
df$value <- df$X1 * df$X2
p1 <- ggplot(df, aes(X1, X2)) + geom_tile(aes(fill = value))

p1 + scale_fill_continuous(function(x) base::toupper(x))

# With function shorthand
p1 + scale_fill_continuous(~ base::toupper(.x))

Created on 2021-01-11 by the reprex package (v0.3.0)

Something like this would make code snippets more portable. In my case, I have a function that makes a plot and I had to define the title outside the plot depending on the function arguments. With this, I could've defined a global dictionary which translates column names to actual (human readable) variable names and re-use that all over my report.

To produce the code above I just added this

  if (is.formula(name)) {
    name <- as_function(name)
  }

  if (is.function(name)) {
    make_title <- name
    name <- waiver()
  } else {
    make_title <- super$make_title
  }

before this line:

https://github.com/tidyverse/ggplot2/blob/master/R/scale-.r#L107

And then added make_title = make_title to the ggproto construction.

All tests pass, so changing this should be safe.

eliocamp avatar Jan 11 '21 20:01 eliocamp

I like the idea. One more thing to consider is how to handle make_sec_title(). I think we can just assign the same as make_title().

https://github.com/tidyverse/ggplot2/blob/37eb64de719c6f60c9f467f5530d59d2a477ee07/R/scale-.r#L515-L517

yutannihilation avatar Jan 11 '21 23:01 yutannihilation

I saw that but didn't know what it did, so I didn't dare to touch anything. Is that for the second axis, if there is one?

eliocamp avatar Jan 11 '21 23:01 eliocamp

On this topic: For any new function you add, or any related existing function where you figure out what it does, please add a few comments to the code in scale-.r so in the future we won't have these questions about what certain parts of the API actually do.

clauswilke avatar Jan 11 '21 23:01 clauswilke

Is that for the second axis, if there is one?

Yes, I guess so, but I too don't know well about the Scale-related things. Honestly, I have no idea what is the case when there's no secondary.axis but the plot needs a secondary title...

https://github.com/tidyverse/ggplot2/blob/37eb64de719c6f60c9f467f5530d59d2a477ee07/R/scale-continuous.r#L144-L150

The mechanism was added by #1868, so we might find some clue there.

yutannihilation avatar Jan 11 '21 23:01 yutannihilation

Well, maybe @thomasp85 remembers what he did there. :-)

clauswilke avatar Jan 12 '21 00:01 clauswilke

I think I got it. I need to add a similar trick here:

https://github.com/tidyverse/ggplot2/blob/37eb64de719c6f60c9f467f5530d59d2a477ee07/R/axis-secondary.R#L122-L129

With that change, now it works also for secondary axis.

library(ggplot2)

df <- expand.grid(X1 = 1:10, X2 = 1:10)
df$value <- df$X1 * df$X2
p1 <- ggplot(df, aes(X1, X2)) + geom_tile(aes(fill = value))


p1 + scale_x_continuous(~ base::tolower(.x), 
                        sec.axis = sec_axis(trans = identity, 
                                            name = ~ gsub("x", "xx", .x))) +
  scale_fill_continuous(~ base::toupper(.x))

Created on 2021-01-11 by the reprex package (v0.3.0)

How should I test this new functionality? My default idea is to create visual tests, but there must be a more specific way.

eliocamp avatar Jan 12 '21 01:01 eliocamp

@clauswilke you think too highly of my memory 😄

But I'm sure I had my reasons...

thomasp85 avatar Mar 24 '21 10:03 thomasp85

@eliocamp are you in the process of preparing a PR?

thomasp85 avatar Mar 24 '21 10:03 thomasp85

and do your changes also work with p + labs(x = ~toupper(.x)), i.e. when setting label names outside of the scale?

thomasp85 avatar Mar 24 '21 10:03 thomasp85

and do your changes also work with p + labs(x = ~toupper(.x)), i.e. when setting label names outside of the scale?

It doesn't, and for it to fully work I'd need to change a lot of how labs work, I think.

Right now labs() changes the plot$labels variable, which is a character vector, so it cannot contain a function. A workaround is to apply the function to the label during update_labels() here:

https://github.com/tidyverse/ggplot2/blob/de0b9e371f7067927348e8cf562cd5d8348fe152/R/labels.r#L13-L17

However, if labs() is called before the relevant aesthetic is present, it doesn't work:

library(ggplot2)
df <- expand.grid(X1 = 1:10, X2 = 1:10)
df$lowercase <- df$X1 * df$X2

ggplot(df, aes(X1, X2)) + 
  geom_tile(aes(fill = lowercase)) +
  labs(fill =  ~base::toupper(.x))  

ggplot(df, aes(X1, X2)) + 
  labs(fill =  ~base::toupper(.x))  +
  geom_tile(aes(fill = lowercase)) 

Created on 2021-03-24 by the reprex package (v1.0.0)

To solve this labs() would need to either add a new scale other modify the existing scale, which will probably break a lot of existing behaviour.

eliocamp avatar Mar 24 '21 14:03 eliocamp

I would hold off on touching labs() until we get to #4269. Both @yutannihilation and I had reasonable prototypes for addressing this issue. But that's for a major feature release, not a patch release.

clauswilke avatar Mar 24 '21 14:03 clauswilke

Any advice on how to add tests for this feature? Should it be a visual test or there's a better way of checking the text of axis and legends?

eliocamp avatar Mar 24 '21 15:03 eliocamp

Any implementation of this could include a fix for #4631

Sounds like we could ignore labs() for now, and tackle that as a part of a bigger fix.

@eliocamp I think you should be able to find some part of the data structure that exposes the actual name and test that. I don't remember where it is, but if I was writing the test, I would start by doing a View(p) and str(p) and looking for the label.

hadley avatar Mar 14 '22 21:03 hadley