Implement a safer alternative to funs()

[krlmlr]

Using anonymous functions or shortcuts from purrr. From discussions in #3094 and #3368.

# Current
iris %>% summarise_if(is.numeric, funs(mean))
# Proposed
iris %>% summarise_if(is.numeric, mean)
iris %>% summarise_if(is.numeric, ~mean(., na.rm = TRUE))
iris %>% summarise_if(is.numeric, mean = ~mean(., na.rm = TRUE))

Also fixes scoped functions for tbl_lazy, see #3594:

library(DBI)
library(dplyr, warn.conflicts = FALSE)

con <- dbConnect(RSQLite::SQLite(), ":memory:")
dbWriteTable(con, "mtcars", mtcars)

tbl(con, "mtcars") %>%
  dplyr::summarise_all(~ any(is.na(.)))
#> Error in (function (..., .x = ..1, .y = ..2, . = ..1) : object 'mpg' not found

Easiest to do with new verbs. I think we can still continue supporting funs() for some time.

Need to keep in mind updating and improving the documentation.

[lionel-]

When users supplies functions I think we can check if they are identical to base/stats variants and in that case create calls so the hybrid evaluator can pick them up.

[lionel-]

I think there would be no new verbs needed. We'd just accept functions and lists (maybe we do already).

[hadley]

And formulas, which I don't think we do currently.

[hadley]

We need to prioritise this for the next release because the current behaviour are so inconsistent (particularly across funs and any_vars, and then compared to purrr)

[hadley]

As part of this we should consider the argument names to filter_if and filter_at, because the current args are confusing

[hadley]

If we switch to list(), we'll need a new helper as suggested by @lionel-: something that returns the name of a hybridised function when given the function object itself. @romainfrancois how many hybridised functions do we have? If it's only a handful, that's probably a good approach.

That way .funs could be either a function, a formula, or a list of functions/formulas. We'd need to think about whether to support purrr style helpers: the advantage is greater consistency, but we can't use as_mapper() directly because for ~ style functions we want to generate expressions, not anonymous functions.

Another approach would be modify funs() to handle a call to ~ specially, and to deprecate any call that isn't :: or ~. In the first release, we could automatically translate with a message so that the previous behaviour was preserved, but we steered people towards the new approach.

[romainfrancois]

I'm about to work on #3526 so I'll have a clearer picture in a few days, it's not that many functions, but part of what I'm after with #3526 is that it will be easier to make new ones.

[krlmlr]

Would it work if we translate e.g. summarize_if(..., mean) to summarize_if(..., ~mean(.)) internally? That would make the proposed function -> hybrid map unnecessary IMO.

We could implement funs() so that it always returns a named list of formulas, and allow lists (and formulas and functions) in the interface for the scoped functions. I'm in favor of keeping funs() around for the next release.

[hadley]

@romainfrancois will make a function that returns a character vector of the hybridised function so we can compare individually for equality.

[hadley]

Lets calve out hybridising anonymous functions created by ~ into another issue.

Then I think this issue can be resolved with three steps:

• soft-deprecate funs(); i.e. warn once per session
• create new checking code
• updated examples to use list()

[romainfrancois]

#3888 implements the second •

library(tidyverse)

trace(summarise_at, exit = quote(print(funs)))
#> Tracing function "summarise_at" in package "dplyr"
#> [1] "summarise_at"

fns <- map(list(m1 = mean, m2 = ~mean(.)), rlang::as_function)
summarise_at(mtcars, vars(cyl), fns)
#> Tracing summarise_at(mtcars, vars(cyl), fns) on exit 
#> $m1
#> <quosure>
#>   expr: ^<function(x, ...) UseMethod("mean")>(cyl)
#>   env:  global
#> 
#> $m2
#> <quosure>
#>   expr: ^<S3: rlang_lambda_function>(cyl)
#>   env:  global
#>       m1     m2
#> 1 6.1875 6.1875

Created on 2018-10-10 by the reprex package (v0.2.1.9000)

[krlmlr]

Warn if funs() is called from .GlobalEnv ?

[romainfrancois]

This is essentially done. To be finalized in #4125

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/