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

Variant of invoke when both functions and params vary #125

Closed
hadley opened this issue Nov 10, 2015 · 10 comments

Comments

@hadley
Copy link
Member

commented Nov 10, 2015

Currently:

library(dplyr)
library(purrr)

df <- data_frame(
  r = c("runif", "rpois", "rnorm"),
  params = list(
    list(n = 10),
    list(n = 5, lambda = 10),
    list(n = 10, mean = -3, sd = 10)
  )
)

map2(df$r, df$params, ~ do.call(.x, .y))
map2(df$r, df$params, ~ lift_dl(match.fun(.x))(.y))

How can we do better?

@lionel-

This comment has been minimized.

Copy link
Member

commented Nov 10, 2015

maybe add a .params argument to invoke() that should be the same size as .x?
The dots would be shared arguments and the .params would be specific arguments.

Then it should work with map_n() when the data frame has .x and .params columns, but we could add a .unnamed argument as in lift_dl() and lift_dv(), to match by column position instead.

df %>% map_n(invoke)
@lionel-

This comment has been minimized.

Copy link
Member

commented Nov 10, 2015

by the way, I wonder if it would be nice to have an alias for map_n() that reflects that it's a vectorised version of map_call() over the "rows" of a rectangular list.

This would have heuristic value and would help users understand how map_n() can be used. I'm not sure what name though. cc @jennybc who enjoyed a "aha" moment when she realised map_n() can be used with data frames without using lift() :)

@hadley

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2015

I wonder if map_call() is the right name - it doesn't seem evocative given (false) parallel with map_int(), map_lgl() etc

@hadley

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2015

It feels very natural to me to have invoke_n():

invoke_n <- function(.f, .x, ...) {
  map2(.f, .x, function(f, x) {
    do.call(f, c(x, list(...)))
  })
}
invoke_n(df$r, df$params)

Maybe it's more natural for invoke to be the scalar vector of this? (i.e. map_call()). That would lose the current behaviour of invoke, but we could bring that back by allowing .x to be NULL. That gives:

invoke <- function(.f, .x, ...) {
  if (is.null(.x)) {
    .f(...)
  } else {
    stopifnot(is.list(.x))
    do.call(.f, c(.x, list(...)))
  }
}

invoke_n <- function(.f, .x, ...) {
  if (is.null(.x)) {
    lapply(.f, function(f) do.call(f, list(quote(...))))
  } else {
    map2(.f, .x, invoke)  
  }
}

invoke(runif, list(n = 10))
invoke_n(list(runif, rnorm), list(list(n = 10), list(n = 5)))
invoke_n(list(runif, rnorm), NULL, n = 5)

invoke("runif", list(n = 10))
invoke_n(list("runif", "rnorm"), list(list(n = 10), list(n = 5)))
invoke_n(list("runif", "rnorm"), NULL, n = 5)

That feels pretty natural to me.

We need to note that the semantics for .f are different: a character vector gives the name of a function. (And there's no support for ~, which we could add)

@lionel-

This comment has been minimized.

Copy link
Member

commented Nov 10, 2015

that's brilliant.

Do we need another suffix? _n usually conveys that we're working with a list of lists. Would invoke_v to signal vectorisation over .f be better?

@hadley

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2015

@lionel- I quite like _n - it's not precisely right but it conveys the right feel to me (plus .x will be a list of lists)

@hadley

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2015

One downside is that we'll need invoke_n_int etc once we have map2_int() etc.

Maybe it's actually invoke() and invoke_map() ?

@lionel-

This comment has been minimized.

Copy link
Member

commented Nov 10, 2015

Or is it a zip_invoke()? since we pair each of .f with one of .x?

@hadley

This comment has been minimized.

Copy link
Member Author

commented Nov 10, 2015

I think that would suggest invoke2() (analogous to map2()), but then that makes you think invoke() is like map(), but it's not vectorised.

@hadley hadley closed this in debe5ed Nov 10, 2015

@lionel-

This comment has been minimized.

Copy link
Member

commented Nov 10, 2015

yes actually in Haskell they have zipWith instead of map2.

I think map2() and map_n() are a bit confusing because they don't convey well the idea that they work with "rows", that is, the elements of each list that occur at the same position.

So that's a naming pattern to consider while purrr's API is still consolidating. Though it's by no means a bad thing to stay with the status quo.

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