expr_substitute() does substitute double sided formula

[romainfrancois]

library(dplyr, warn.conflicts = FALSE)

tibble(x = 1:10) %>% 
  mutate(across(x, ~ case_when(
    . < 5 ~ 5L, 
    TRUE  ~ .
  )))  
#> # A tibble: 10 x 1
#>        x
#>    <int>
#>  1     5
#>  2     5
#>  3     5
#>  4     5
#>  5     5
#>  6     6
#>  7     7
#>  8     8
#>  9     9
#> 10    10

^{Created on 2021-06-10 by the reprex package (v2.0.0)}

[romainfrancois]

only remaining problem after revdepcheck on this pr is :

> revdepcheck::cloud_details(, "imfr")
══ Reverse dependency check ═════════════════════════════════════════════════════════════ imfr 0.1.9.1 ══

Status: BROKEN

── Newly failing

x checking tests ... ERROR

── Before ───────────────────────────────────────────────────────────────────────────────────────────────
0 errors ✓ | 0 warnings ✓ | 0 notes ✓

── After ────────────────────────────────────────────────────────────────────────────────────────────────
> checking tests ... ERROR
  See below...

── Test failures ────────────────────────────────────────────────────────────────────────── testthat ────

> library(testthat)
> library(imfr)
> 
> test_check("imfr")
  |======================================================================| 100%
  |======================================================================| 100%
  |======================================================================| 100%
  |======================================================================| 100%
  |======================================================================| 100%
══ Failed tests ════════════════════════════════════════════════════════════════
── Failure (test-next.R:1:1): (code run outside of `test_that()`) ──────────────
`imf_data(...)` threw an error.
Message: GnuTLS recv error (-110): The TLS connection was non-properly terminated.
Class:   simpleError/error/condition
Backtrace:
     █
  1. ├─testthat::expect_error(...) test-next.R:1:0
  2. │ └─testthat:::quasi_capture(...)
  3. │   ├─testthat:::.capture(...)
  4. │   │ └─base::withCallingHandlers(...)
  5. │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  6. ├─imfr::imf_data(...)
  7. │ └─imfr:::imf_data_one(...)
  8. │   └─imfr:::download_parse(URL)
  9. │     ├─`%>%`(...)
 10. │     └─httr::RETRY("GET", URL, user_agent(""), progress(), times = times)
 11. └─httr::content(., type = "text", encoding = "UTF-8")
 12.   ├─base::stopifnot(is.response(x))
 13.   └─httr:::is.response(x)

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 1 ]
Error: Test failures
Execution halted

1 error x | 0 warnings ✓ | 0 notes ✓

This is unrelated to the business of this pull request. I can have a look independently.

[lionel-]

I was wondering if this would get us in trouble with model fitting e.g. inside summarise(). However the conflict between syntax seems sufficiently obvious?

df <- tibble(
  y = 1:5,
  g = c(1, 1, 2, 2, 2),
  w = tibble(w1 = rnorm(5), w2 = rnorm(5)),
  x = tibble(x1 = rnorm(5), x2 = rnorm(5))
)

df %>%
  group_by(g) %>%
  summarise(
    across(w:x, ~ list(lm(y ~ ., data = .x)))
  )

[romainfrancois]

Yeah, I think that's fine. I don't do much modeling, but IIRC there are fewer cases of formula with right side only. In any case, we can always recognize these and guide towards using a real named function, that improves readability anyway.

And (even if that's not full proof) no CRAN package fails because of this change.

I'll go forward.