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

Strange variable handling inside functions #58

Closed
smped opened this issue Feb 27, 2024 · 6 comments
Closed

Strange variable handling inside functions #58

smped opened this issue Feb 27, 2024 · 6 comments

Comments

@smped
Copy link
Contributor

smped commented Feb 27, 2024

Hi @jtlandis ,

Great work getting ggside ready for the breaking ggplot2 changes! That was such brilliant fast work.

Unfortunately, I've found a weird quirk in the new version & I don't know if it's connected to their new updates. In short, passing values to ggside geoms inside a function has gone a bit strange.

p <- mtcars |>
    dplyr::mutate(cyl = as.factor(cyl)) |>
    ggplot(aes(mpg, hp)) +
    geom_point() 
## Looks about right
p

.addSide <- function(p, my_alpha) {
    message("my_alpha is: ", my_alpha)
    p + geom_ysideboxplot(
        aes(x = cyl, y = hp, fill = cyl), orientation = "x",
        alpha = my_alpha
    )
}
.addSide(p, 0.5)

To which I get Error in eval(call, envir = env) : object 'my_alpha' not found, which is clearly a bit of a glitch because it's there in the message line. For context, the function I've just disabled this behaviour in is at https://github.com/smped/extraChIPs/blob/devel/R/plotPairwise.R. It did used to work rather nicely until this update.

I'm sure you've spotted it too but, on Ubuntu 20.04 at least, the old warnings have made a comeback. Really just a niggle as it's not causing my tests or checks to fail

Warning messages:
1: In Ops.factor(x, range[1]) : '<' not meaningful for factors
2: In Ops.factor(x, range[2]) : '>' not meaningful for factors
@jtlandis
Copy link
Owner

Thank you for reporting!
I'm surprised that there are issues with extraChIPs considering nothing came up in revdep checks. I'll have to investigate the logs further.

I'll see if I can get a hot fix and a new unit test up

jtlandis added a commit that referenced this issue Feb 27, 2024
…ors from `scale$map(<vec>)`. Fixes resurgence of #33 as seen in #58
@jtlandis
Copy link
Owner

hello @smped, please see if the latests commits (f04fee5) main branch builds and passes tests with extraChIPs package.

@jtlandis
Copy link
Owner

It seems that revdepcheck tends to timeout, (no matter how much time I give to timeout =). I was able to complete a R CMD check on the .tar.gz for extraChIPs but I would still appreciate a double check @smped before I push this hotfix to CRAN

@smped
Copy link
Contributor Author

smped commented Feb 28, 2024

Hi @jtlandis ,

Amazing work again! Thanks for keeping on top of things like you have. My local tests are now passing, which is great news.

All the best

@jtlandis
Copy link
Owner

Alright, I'll try and resubmit this afternoon.

jtlandis added a commit that referenced this issue Feb 28, 2024
@jtlandis
Copy link
Owner

@smped CRAN was fairly kind and uploaded the hotfix

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