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

scale_y_continuous() fails if inside a function called via double colon pkg::fun(...) #5436

Closed
averissimo opened this issue Sep 26, 2023 · 1 comment · Fixed by #5443
Closed
Labels
bug an unexpected problem or unintended behavior

Comments

@averissimo
Copy link

I found a problem with recent PR #5343 when scale_y_continuous was called inside a package function via double colon

I expected the function not to throw an error

Here is the code to reproduce the bug:

Please note that the problem is not with {tern} itself, as the call without double colon succeeds

install.package("tern")
remotes::install_github("tidyverse/ggplot2")

library(tern)

# example from tern::g_lineplot function
library(nestcolor)

adsl <- tern_ex_adsl
adlb <- tern_ex_adlb %>% dplyr::filter(ANL01FL == "Y", PARAMCD == "ALT", AVISIT != "SCREENING")
adlb$AVISIT <- droplevels(adlb$AVISIT)
adlb <- dplyr::mutate(adlb, AVISIT = forcats::fct_reorder(AVISIT, AVISITN, min))
g_lineplot(adlb, adsl, subtitle = "Laboratory Test:")

# This call falls on if clause `... && !startsWith(as.character(call[[1]]), "scale_")` as it has length = 3
tern::g_lineplot(adlb, adsl, subtitle = "Laboratory Test:")
@teunbrand teunbrand added the bug an unexpected problem or unintended behavior label Sep 26, 2023
@teunbrand
Copy link
Collaborator

Thanks for the report! I can reproduce this bug. A more minimal reprex:

# Current main branch
devtools::load_all("~/packages/ggplot2")
#> ℹ Loading ggplot2

env <- new.env()
env$foo <- function() {
  ggplot() + 
    geom_point(aes(x = "A", y = 2)) +
    scale_x_continuous()
}

env$foo()
#> Error in is.null(call) || !startsWith(as.character(call[[1]]), "scale_"): 'length = 3' in coercion to 'logical(1)'

Created on 2023-09-26 with reprex v2.0.2

teunbrand added a commit to teunbrand/ggplot2 that referenced this issue Sep 27, 2023
teunbrand added a commit that referenced this issue Oct 2, 2023
* Fix #5436

* Add tests
shajoezhu pushed a commit to insightsengineering/tern that referenced this issue Oct 23, 2023
WIP :: parent issue:
insightsengineering/nestdevs-tasks#7

Supersede:
* #957

### 🔴 Checklist for PR Reviewer

[![Scheduled
🕰️](https://github.com/insightsengineering/tern/actions/workflows/scheduled.yaml/badge.svg?branch=verdepcheck_action)](https://github.com/insightsengineering/tern/actions/workflows/scheduled.yaml)
_(~~max strategy fails due to
tidyverse/ggplot2#5436 corrected upstream)_

- [ ] Tag yourself next to this repo on
insightsengineering/nestdevs-tasks#7
- [ ] Package versions are the same or higher than `main`
- [ ] Package list is the same
  - Only exception is `rmarkdown` (may have been removed on `Suggests`)
- [ ] All packages in `Imports`, `Depends` & `Suggests` are in new
section `Config/Needs/verdepcheck`
- [ ] Added entry to `NEWS.md`
- [ ] Last `scheduled.yaml` action was run succesfully _(all 4
strategies)_
- important: it's not the last commit, it's the one that runs 4
`Scheduled 🕰️ / Dependency` actions
- [ ] `scheduled.yaml` SHOULD NOT have any push on any branches

### 🔴 What's needed before merging?

This PR depends on some upstream changes that need to be
finalized/merged before being ready to review.

#### Change in code

* `verdepcheck.yml` action (see comments)
  - [x] Remove `on: push` section 
  - [x] Change branch to main

#### PRS

- [x] verdepcheck
  * insightsengineering/verdepcheck#24
  * insightsengineering/verdepcheck#26
- [x] verdepcheck-action
  * insightsengineering/r-verdepcheck-action#16

### Changes description

* Adds minimum version for packages `DESCRIPTION`
* Adds `Config/Need/verdepcheck` section in `DESCRIPTION`
* Updates verdepcheck action

---------

Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com>
Co-authored-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants