Skip to content
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

Compatibility with custom geoms #27

Closed
carlschmidt26 opened this issue Oct 26, 2020 · 4 comments
Closed

Compatibility with custom geoms #27

carlschmidt26 opened this issue Oct 26, 2020 · 4 comments

Comments

@carlschmidt26
Copy link

Hi,

I really like your package. Your effort is really much appreciated!

However when combining my custom geom (e.g. geom_mycustom()) with new_scale_fill() I encounter an problem, which I'd like to point out in the following
The problem is that the setup_data() function of the associated stat looks like this:

setup_data = function(data, params) {
  data <- plyr::ddply(data, c("x", "fill"), within,
                      # Assign the range of `y` to the newly created column `range`.
                      assign("range", list(range(y))))
}

The problem is that the column fill becomes fill_new in the second iteration due to new_scale_fill() when the whole plot is built again. This results in the following error:

Error in unique.default(x) :
unique() applies only to vectors

A solution to that is to simply change setup_data as follow:

setup_data = function(data, params) {
  fill_name <- str_subset(names(data), "fill")
  data <- plyr::ddply(data, c("x", fill_name), within,
                      # Assign the range of `y` to the newly created column `range`.
                      assign("range", list(range(y))))
}

Just out of curiosity an because I have a few more places where I use plyr::ddply() I was wondering if there might be another way to resolve this problem.

Happy to hear from you.
Best regards

@eliocamp
Copy link
Owner

Thanks. Do you have a reproducible example? ggnewscale is supposed to mimic your solution, but it's possible it needs to do more.

@carlschmidt26
Copy link
Author

Thanks for the quick reply.
Here is an example which throws the error mentioned above:

library(tidyverse)
library(ggnewscale)

# Manipulate `setup_data()` of `StatYdensity` and store the object as `StatYdensity2`.
StatYdensity2 <- ggproto("StatYdensity2", StatYdensity,
                         setup_data = function (data, params) {
                           data <- plyr::ddply(data, c("x", "fill"), within,
                                               # Assign the range of `y` to the newly created column `range`.
                                               assign("range", list(range(y))))
                           data
                         })

set.seed(5)

df <- data.frame(
  x = floor(runif(100, min=1, max=5)),
  y = floor(runif(100, min=1, max=10)),
  gender = c("female", "male")[floor(runif(100, min=1, max=3))],
  fill = floor(runif(100, min=1, max=5))
)

df %>% ggplot(aes(x, y, fill = gender, group = interaction(x, gender))) + 
  # Call `geom_violin()` using the manipulated stat `StatYdensity2`.
  geom_violin(stat = "ydensity2") + 
  new_scale_fill() + 
  geom_point(aes(x, y, fill = fill), shape = 21, inherit.aes = F)

Note that the call of plyr::ddply() does not make that much sense in this example, however as pointed out earlier, this is the source of the error.

Either applying the solution of the original post or simply calling geom_violin() instead of geom_violin(stat = "ydensity2") works like a charm. plyr::ddply() just isn't happy about the renaming of the variables that takes places due to the call of new_scale_fill().

At this point I think it's best to use the solution is already described. I just thought it's not that unusual to use plyr::ddply() in geoms at some point in general.

Looking forward to hearing about your thoughts on this.

@eliocamp
Copy link
Owner

eliocamp commented Nov 5, 2020

Great! This now should be fixed in the dev version you can install with remotes::install_github("eliocamp/ggnewscale@dev"). Let me know if it works :)

Thanks for the report and the example (I added it as a test).

@carlschmidt26
Copy link
Author

Great, thank you very much.
I am quite busy at the moment, but I will give it a try as soon as can in the next days!

Thanks again and all the best!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants