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

Invalid flagging of faulty ... usage (part 2) #72

Closed
DavisVaughan opened this issue Jan 12, 2021 · 1 comment
Closed

Invalid flagging of faulty ... usage (part 2) #72

DavisVaughan opened this issue Jan 12, 2021 · 1 comment

Comments

@DavisVaughan
Copy link
Contributor

Follow up of #62, after receiving this comment #62 (comment) reproduced below:

library(purrr)
library(future)

catcher <- function(x) {
  map(x, ~list(...))
}

catcher(list(1:2, 1:3))
#> [[1]]
#> [[1]][[1]]
#> [1] 1 2
#> 
#> 
#> [[2]]
#> [[2]][[1]]
#> [1] 1 2 3

future({
  catcher(list(1:2, 1:3))
})
#> Error in getGlobalsAndPackages(expr, envir = envir, tweak = tweakExpression, : Did you mean to create the future within a function?  Invalid future expression tries to use global '...' variables that do not exist: {; catcher(list(1:2, 1:3)); }

Created on 2021-01-12 by the reprex package (v0.3.0.9001)

It looks like the dots are being returned as an NA that gets wrapped with the DotDotDotList class.
a05c813#diff-7b1d4e2796df08171808d5d5341fadf8c78a22cc2ee1c2874cc4d3ebb2504dcbR233

But then in future::getGlobalsAndPackages(), it checks to make sure that the dots are a list, and errors otherwise (as we see above) https://github.com/HenrikBengtsson/future/blob/0755d6f30219f596f27f948a1e1df1a6ddc71ed0/R/globals.R#L174

Maybe instead of NA, it should be setting ddd to list() in some cases like this one? That is my initial guess.

@HenrikBengtsson
Copy link
Owner

Thxs. Finally got around to this one. I decided to fix this for now in future, cf. HenrikBengtsson/future@89d28d8.

I think the original reason to return an structure(NA, class = "DotDotDotList") was to be conservative and catch cases like this that I couldn't foresee at the time. I think the proper, long-term fix is to not return a globals[["..."]] element at all, because it's not a global. I'll create a separate issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants