Skip to content

Infinite Improbability default_args #64

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.

Sign up for GitHub

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

Jump to bottom
Open
MikeBadescu opened this issue Mar 17, 2018 · 2 comments · May be fixed by #88
Open

Infinite Improbability default_args #64

MikeBadescu opened this issue Mar 17, 2018 · 2 comments · May be fixed by #88
Labels
bug an unexpected problem or unintended behavior

Comments

@MikeBadescu
Copy link

Jim,

A possible bug:

Line 123: lapply(default_args, eval, envir = environment()))

If default_args contains one of the already defined symbols in memo_f or its enclosed environment (e.g., _f), it will use this value instead of the default argument specified in the function definition. Improbable example:

# based on test: "argument names don't clash with names in memoised function body"
library(memoise)

f <- function(
  # note that `_f` is not included as argument
  `_cache`, `_additional`,
  mc, encl, called_args, default_args, args, hash, res, xtra = `_f`
) list(`_f`, `_cache`, `_additional`, mc, encl, called_args, default_args, args, hash, res, xtra)
f_mem <- memoise(f)

`_f` <- 100
(unlist(f(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 100   1   2   3   4   5   6   7   8   9 100
(unlist(f_mem(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 100   1   2   3   4   5   6   7   8   9 100
# looks good

`_f` <- 200
(unlist(f(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 200   1   2   3   4   5   6   7   8   9 200
(unlist(f_mem(1, 2, 3, 4, 5, 6, 7, 8, 9)))
# [1] 100   1   2   3   4   5   6   7   8   9 100
# does not match

I believe using

envir = environment(encl$`_f`)

would solve this problem as there is no other eval that takes place inside the memo_f frame / enclosing environment. (All the other tests pass.)

Q: We still have eval of _additional on Line 127 which takes place in encl --> there might be a possible (but improbable) conflict of ... formula with names in encl: _f, _additional and _cache. Is this correct?
@jimhester jimhester added the bug an unexpected problem or unintended behavior label May 4, 2018
@bluaze
Copy link

bluaze commented May 8, 2019 •
edited
Loading

evaluating default arguments in environment(encl$`_f`) would have problem with arguments whose default values are other arguments, e.g.

f <- function(x = y, y) x + y
fm <- memoise(f)
fm(y = 1)

@bluaze
Copy link

bluaze commented May 8, 2019 •
edited
Loading

if we could find a way to "snapshot" the environment at the beginning, then the snapshot could be used to evaluate the default arguments

@bluaze bluaze linked a pull request May 10, 2019 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

evaluate default args in a dedicated environment bluaze/memoise
3 participants
@jimhester @MikeBadescu @bluaze