tidyverse / purrr

A functional programming toolkit for R
https://purrr.tidyverse.org/
Other
1.28k stars 272 forks source link

`reduce_impl()` needs to name `error_call` something like `purrr_error_call` #1002

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 2 years ago

Like with map_(..error_call), we need to rename the error_call arg of reduce_impl() to something less liable to conflict with user arguments passed through ...

In tidyr, with dev purrr installed some snapshot tests change and I see this incorrect error call of fn()

library(tidyr)

df <- tibble(x = list(1:2), y = list(1:3))

# Used to say `unchop()` in the call
unchop(df, c(x, y))
#> Error in `fn()`:
#> ! In row 1, can't recycle input of size 2 to size 3.

#> Backtrace:
#>     ▆
#>  1. └─tidyr::unchop(df, c(x, y))
#>  2.   └─tidyr:::df_unchop(cols, ptype = ptype, keep_empty = keep_empty)
#>  3.     └─purrr::reduce(x_sizes, unchop_sizes2, error_call = error_call)
#>  4.       └─purrr:::reduce_impl(.x, .f, ..., .init = .init, .dir = .dir)
#>  5.         └─tidyr (local) fn(out, elt, ...)
#>  6.           └─cli::cli_abort(...)
#>  7.             └─rlang::abort(...)

This is due to df_unchop() calling reduce() and passing error_call through, but it gets captured early by reduce_impl() and isn't passed through to unchop_sizes2().

I suggest that we rename all of these cases to purrr_error_call and .purrr_error_call (in cases when we dot the args, for consistency) to eliminate potential conflicts and have a consistent way to handle this

We should probably see if there are other places this can come up

hadley commented 2 years ago

We should probably do this, but maybe in tidyr we should also use an anonymous function rather than passing all the arguments through ...?

DavisVaughan commented 2 years ago

Yea I think so too. I think this is the second case we've seen where providing an anonymous function is probably better than providing extra args through ...? The other one being map()? I wonder if we want to discourage using ... anywhere in the docs

hadley commented 2 years ago

You mean like this? 😛

Screen Shot 2022-11-14 at 10 19 25