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

Need to release bindings created when calling functions #4

Open
hmansell opened this issue Jul 31, 2012 · 4 comments
Open

Need to release bindings created when calling functions #4

hmansell opened this issue Jul 31, 2012 · 4 comments
Labels
Type: Bug Something is broken!

Comments

@hmansell
Copy link
Contributor

Currently, bindings are set in R forever and never released

@glchapman
Copy link

This may be a non-starter for performance (or other) reasons, but I wonder if you've considered changing callFunc so that it evals an anonymous function with the real call as the body and the generated symbols as formal parameters, then invoking the anonymous function with the appropriate values as arguments (i.e., modeling a let using a lambda). It seems like this would eliminate the need for the top-level bindings.

@hmansell
Copy link
Contributor Author

Can you elaborate on what you expect that to achieve? Are you talking about the top-level bindings in R? They are needed to pass argument values through (in cases where they cannot be robustly passed through as literals.

@glchapman
Copy link

Here's what I had in mind. I changed RInterop so that it still generates tempSymbols, but no longer regsters then with the Engine. I then changed the end of callFunc to

    let callargs = String.Join(", ", argList)
    if tempSymbols.Count = 0 then
        let expr = sprintf "%s::`%s`(%s)" packageName funcName callargs
        eval expr
    else
        let formals = String.Join(", ", Seq.map fst tempSymbols)
        let expr = sprintf "function(%s) {%s::`%s`(%s)}" formals packageName funcName callargs
        let func = (eval expr).AsFunction()
        func.Invoke(Seq.map snd tempSymbols |> Seq.toArray)

I haven't run into any problems with this change so far, but I've not extensively tested it nor have I tried to measure performance.

@hmansell
Copy link
Contributor Author

OK, I think I understand, but a pull-request with fully working code would make it easier to follow.

Does this with with varargs (... args in R)? It's important that those get passed, and the specified name is passed too (for example, when constructing something like a data.frame).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something is broken!
Projects
None yet
Development

No branches or pull requests

5 participants