-
Notifications
You must be signed in to change notification settings - Fork 324
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
[SparkR-237, 238] Fix cleanClosure by including private function checks in package namespaces. #229
base: master
Are you sure you want to change the base?
Conversation
hlin09
commented
Mar 20, 2015
- Fixes 237 by including private function checks in package namespaces.
- Add a test for this.
@piccolbo Would be great if you could help test this patch. |
Will do |
Does not pass. Fails a little later, same way. Details to follow shortly. |
This is the first failure I got
So this is progress, because kv2rdd.list is private and is found but keys.spark is also private and is not found. I patched the code to fully qualify that name and it moves to a later point of failure:
Now this is getting complicated. lazy_eval is in package lazyeval, imported by plyrmr (just like SparkR). as.lazy is an exported function in that package. So my initial idea about this being an issue with private functions only is not the correct one. |
Thanks @piccolbo for reporting. Let me do more debugging on this later today. |
I think it's a good idea because also of SPARKR-238. The instructions talk I think if you cut it to the chase you just need to do library(devtools) We don't normally give these instructions because we need regular users to Then R CMD check path-to-plyrmr will repro 237 On Fri, Mar 20, 2015 at 4:28 PM, Shivaram Venkataraman <
|
@piccolbo Thanks for the helpful instructions. I have just done some tests. Please try this patch and let me know if it works. |
The tests pass the original error point, but fail elsewhere. From the error message, it seems an instance of SparkR-238, not 237. My suggestion is that we can assume 237 fixed and focus on 238, but maybe wait to close it until all plyrmr tests pass cleanly (I am assuming that all problems will prove to be related to changes in SparkR, which is only a working hypothesis). |