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

compose() does not work with generic functions #629

Closed
MarselScheer opened this issue Jan 30, 2019 · 5 comments

Comments

@MarselScheer
Copy link

commented Jan 30, 2019

In purrr 0.3.0
purrr::compose(t,t,identity)(1:3)
does not match
purrr::compose(t,t)(1:3)
In purrr 0.2.5 they match and I assume that this is the correct behavior

@yutannihilation

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

The problem is compose() now extracts the first function's body. If the body UseMethod(), the value is returned immediately (right?). So, when we add a non-generic function (e.g. identity()) at the end, it works.

Can this line simply be reverted to out <- first_fn(...)?:

out <- !!fn_body(first_fn)

library(purrr)

foo <- function(...) UseMethod("foo")
foo.default <- function(...) message("foo")

purrr::compose(foo)()
#> foo
purrr::compose(foo,foo)()
#> foo
purrr::compose(foo,foo,foo)()
#> foo

body(purrr::compose(foo,foo,foo))
#> {
#>     out <- {
#>         UseMethod("foo")
#>     }
#>     fns <- list(function (...) 
#>     UseMethod("foo"), function (...) 
#>     UseMethod("foo"))
#>     for (fn in fns) {
#>         out <- fn(out)
#>     }
#>     out
#> }

# adding non-generic function works
purrr::compose(foo,foo,foo,function(...) NULL)()
#> foo
#> foo
#> foo
#> NULL

Created on 2019-01-31 by the reprex package (v0.2.1.9000)

MarselScheer added a commit to MarselScheer/simTool that referenced this issue Feb 1, 2019

MarselScheer added a commit to MarselScheer/simTool that referenced this issue Feb 1, 2019

MarselScheer added a commit to MarselScheer/simTool that referenced this issue Feb 1, 2019

@lionel- lionel- added the bug label Feb 25, 2019

@lionel- lionel- changed the title compose sometimes ignores argument compose() does not work with generic functions Feb 25, 2019

@lionel-

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

@yutannihilation Good diagnostic. The goal was to have the same formals list as the original function, so we can have auto-completion of arguments etc. If I can't make it working with generic functions, I'll revert that feature.

@yutannihilation

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Thanks for the background, so we need to construct a function call by matching formals like this?

wrap <- function(f) {
  f_expr <- rlang::enexpr(f)
  
  formal_args <- formals(f)
  idx <- names(formal_args) != "..."
  formal_args[idx] <- rlang::syms(names(formal_args))[idx]

  body <- rlang::expr((!!f_expr)(!!!formal_args))
  rlang::new_function(formals(f), body, rlang::caller_env())
}

wrap(mean)
#> function (x, ...) 
#> mean(x = x, ... = )

wrap(dplyr::group_by)
#> function (.data, ..., add = FALSE, .drop = FALSE) 
#> dplyr::group_by(.data = .data, ... = , add = add, .drop = .drop)

f <- mean
wrap(f)
#> function (x, ...) 
#> f(x = x, ... = )
wrap(f)(1:10)
#> [1] 5.5

Created on 2019-02-26 by the reprex package (v0.2.1.9000)

@lionel- lionel- closed this in c26426a Feb 27, 2019

@lionel-

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

@yutannihilation This approach is not compatible with substitute() / enquo().

@yutannihilation

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

Ah, I see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.