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

option to force evaluation of supplied function in partial to avoid infinite recursion #387

Closed
t-kalinowski opened this issue Sep 30, 2017 · 3 comments

Comments

@t-kalinowski
Copy link
Contributor

commented Sep 30, 2017

Hello,

This makes an infinite recursive loop

f <- function(a = "a", b = "b") format(as.list(environment()))
f <- partial(f, b = "c")
f()

It would be nice to be able to force f inside of partial to avoid this. Something like:

partial2 <- function(fun, ..., .lazy = FALSE) {
  new_args <- if (.lazy)
    eval(substitute(alist(...)))
  else
    list(...)
  formals(fun) <- modifyList(formals(fun), new_args, TRUE)
  fun
}


f <- function(a = "a", b = "b") as.list(environment())
f <- partial2(f, b = "c")
f()
@t-kalinowski

This comment has been minimized.

Copy link
Contributor Author

commented Sep 30, 2017

Also it would be nice if functions returned by partial still allowed users to supply values that override the modified defaults

f <- function(a = "a", b = "b") format(as.list(environment()))
f2 <- purrr::partial(f, b = "c")
f2()
#>   a   b 
#> "a" "c"
f2(b = "user supplied value") 
#> Error in f(b = "c", ...): formal argument "b" matched by multiple actual arguments


f3 <- partial2(f, b = "c")
f3(b = "user supplied value")
#>                     a                     b 
#>                   "a" "user supplied value"
@egnha

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2017

@t-kalinowski Could this be a duplicate of #349? (Your first concern is addressed there.)

Your second suggestion could be convenient. If it were allowed, however, then partial() would no longer correspond to partial function application as it is usually understood (i.e., an operator which reduces the number of arguments of a function). (Unfortunately, in R partial() shouldn't produce a function with a literally contracted argument signature because of the unfortunate use of functions like missing() that inspect a function's formals.)

@t-kalinowski

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2017

Hi @egnha, thanks for taking a look.

I don't think it's a duplicate, because it's possible to address one without addressing the other.
For example, partial2 defined above or partial3 below fixes the infinite recursion issue, but does not address the ls example you provide.

Regarding the usage of missing, that's something I hadn't considered. Good catch. With further thought, I realize that partial2 above also wouldn't work for functions that expect arguments to show up in ....

However, I think these are mostly questions of interface design. An implementation that incorporates the two ideas above, places new arguments in ... correctly, while retaining the current behavior regarding missing is not a technical hurdle, eg:

partial3 <- function(fun, ..., .lazy = FALSE) {
  partially_supplied_args <- if (.lazy)
    eval(substitute(alist(...)))
  else
    list(...)

  force(fun)
  
  function(...) {
    args <- modifyList( partially_supplied_args, eval(substitute(alist(...))), TRUE )
    eval(as.call(c(fun, args)))
  } 
}

f <- function(a = "a", b = "b") {
  if (missing(b))
    paste("b was missing, its value is:", b)
  else
    paste("b was not missing, its value is:", b)
}

f() 
#> [1] "b was missing, its value is: b"

f2 <- purrr::partial(f, b = "c")
f2()
#> [1] "b was not missing, its value is: c"


f <- partial3(f, b = "c")
f()
#> [1] "b was not missing, its value is: c"
f(b = "z")
#> [1] "b was not missing, its value is: z"

But, this still doesn't address #349

ls_all <- partial3(ls, all.names = TRUE)
ls_all()
#> [1] "..."  "args"

@t-kalinowski t-kalinowski changed the title FR: option to force evaluation of ...f in partial to avoid infinite recursion option to force evaluation of supplied function in partial to avoid infinite recursion Nov 15, 2017

@hadley hadley added the feature label Feb 5, 2018

@hadley hadley added the adverb 📚 label May 5, 2018

@lionel- lionel- added bug and removed feature labels Dec 4, 2018

lionel- added a commit to lionel-/lowliner that referenced this issue Dec 20, 2018

@lionel- lionel- closed this in #607 Dec 21, 2018

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