-
Notifications
You must be signed in to change notification settings - Fork 2k
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
"stage()" does not work properly when prefixed with "ggplot2::" #6104
Comments
While I agree #' @importFrom ggplot2 stage |
I agree with your suggestion, but I still hope that |
I think what is happening is that the |
Sorry, with my current ability, I am not capable of understanding the source code you pointed out in a short period of time. However, it can be confirmed that there are indeed some flaws in the evaluation operation of the In addition to the issues I mentioned earlier, I have also noticed the following situations: library(ggplot2)
library(patchwork)
by <- "cut"
p <- ggplot(diamonds, aes(x = cut, y = price))
p1 <- p + geom_violin(aes(colour = .data[[by]], fill = after_scale(color)), trim = FALSE)
p2 <- p + geom_violin(aes(colour = .data[[by]], fill = stage(!!sym(by), after_scale = color)), trim = FALSE)
p3 <- p + geom_violin(aes(colour = .data[[by]], fill = stage(.data[[by]], after_scale = color)), trim = FALSE)
p1 | p2 | p3 The legend of "P3" is inconsistent with "P1" and "P2". Of course, I understand that it can be corrected using |
This particular situation should already be fixed in the current development version of ggplot2. |
To be comprehensive, I think we have three options about the level of fix.
I'd vote for 3 or 1. So, I like #6107 more because the approach of #6108 probably cannot handle the re-export cases. |
Yeah you make a good point. So if we want this: library(ggplot2)
ggplot(mpg, aes(displ, hwy)) +
geom_point(
aes(fill = stage(drv, after_scale = alpha(fill, 0.3))),
shape = 21
) We cannot ggplot(mpg, aes(displ, hwy)) +
geom_point(
aes(fill = do.call("stage", list(start = drv, after_scale = quote(alpha(fill, 0.5))))),
shape = 21
) +
labs(fill = "drv") Nor can we use stage wrappers. Again, points are not transparent. restage <- function(...) stage(...)
ggplot(mpg, aes(displ, hwy)) +
geom_point(
aes(fill = restage(drv, after_scale = alpha(fill, 0.3))),
shape = 21
) +
labs(fill = "drv") Created on 2024-09-16 with reprex v2.1.1 |
I personally holds the position that I know this is a distinction we cannot necessary expect the users to know or understand, but I do think we can make a case for not allowing alternate call constructions |
I don't think that is unreasonable. But if these are just syntax instead of functions, then I think the following should also work: ggplot2::ggplot(ggplot2::mpg, ggplot2::aes(displ, hwy)) +
ggplot2::geom_point(
ggplot2::aes(fill = stage(drv, after_scale = scales::alpha(fill, 0.3))),
shape = 21
)
#> Error in `ggplot2::geom_point()`:
#> ! Problem while computing aesthetics.
#> ℹ Error occurred in the 1st layer.
#> Caused by error in `stage()`:
#> ! could not find function "stage" Created on 2024-09-16 with reprex v2.1.1 |
That I agree on |
Sorry for replying late. While I support @thomasp85's argument, I was thinking about the consistency with the rest of tidyverse. As dplyr allows using these special syntax with data.frame(x = 1) |>
dplyr::select(dplyr::everything())
#> x
#> 1 1 Created on 2024-09-23 with reprex v2.1.1 |
@yutannihilation I agree. The solution is to have |
Thanks for confirming. Then, should we keep this open (although it's not very high priority considering this is the first time we got the request since ggplot2 introduced |
I'm fine with keeping this closed. The argument I made in #6104 (comment) was mostly academic and in practise I think it is fine to support |
Ah, sorry, I misunderstood what #6104 fixed (I thought it was in line with your argument in #6104 (comment)). Then, that should be mostly fine and I agree with keeping this closed, considering the original problem reported in this issue is definitely solved. However, to be precise, " d <- data.frame(x = 1)
# without attaching the namespace and without dplyr::
dplyr::select(d, everything())
#> x
#> 1 1
# without attaching the namespace and with dplyr::
dplyr::select(d, dplyr::everything())
#> x
#> 1 1
library(dplyr, warn.conflicts = FALSE)
# with attaching the namespace and without dplyr::
select(d, everything())
#> x
#> 1 1
# with attaching the namespace and with dplyr::
select(d, dplyr::everything())
#> x
#> 1 1 Created on 2024-09-23 with reprex v2.1.1 |
When I use a function from ggplot2 in my own R package, I need to prefix it with "ggplot2::". I found that, in this case, "stage()" does not work properly!
Here is the code to reproduce the bug:
This is the original example in ggplot2. I just changed "stage" to "ggplot2::stage".
Throws the following error:
The text was updated successfully, but these errors were encountered: