Convert null to NA and vice versa #32
Conversation
Rserve.create().eval() converts NAs to null and Rserve.create().set() converts null to NA. R's NA is still distinguished from R's NULL with this scheme because NA can only be found inside an array (possibly of length 1) while NULL can only be found outside an array. Fix some inconsistent use of whitespace.
|
I'm nervous about this one because it's possible it'll break things inside of RCloud. @gordonwoodhull, can you take a look at this? |
|
Sure. Most likely there are things broken in RCloud, and this will break the workarounds. We have 2 months until feature freeze so I'll check it in soon and see how it fares. Thanks @Kevin-Jin! |
|
Actually, this looks like a complete overhaul and not just about NA. Can you give a brief overview of the other fixes here? |
|
The only changes I've made to src/ were related to NA. I also replaced some typeof checks with underscore.js calls, fixed some inconsistent whitespace, made use of BOOL_TRUE and BOOL_FALSE constants in place of 1 and 0, and added an optional forced_type parameter to Rserve.create().set(). I realize that a lot of changes were made elsewhere in main.js, rserve.js, and rserve.min.js. This is just a consequence of "make" not being run in a long time so a lot of previous commits are reflected in those files now. In order to test out the changes I made in src, I really had no choice but to execute "make". |
|
Ah got it. I can review the changes in isolation. The standard practice is not to commit build artifacts into a PR, but it's easy enough to work around. |
|
Thanks Gordon, that makes sense. I have never committed updates to binaries because that bloats the repository, but now I know not to include the changes to text files resulting from a build. In that case, I'll unstage the changes I made to main.js, rserve.js, and rserve.min.js in that commit and force push the branch. |
|
Sorry I still have not gotten to this. I hope to try it later this week. When I do, I will merge it to a branch on att/rserve since that's a little bit ahead of that fork. I'll pull you into the conversation when I do. |
|
I have merged this and #31 into branch The code looks good, some annoying special cases but that's the nature of this kind of work... |
NA/null -> cscheid/rserve-js#32 handle connection error -> cscheid/rserve-js#31
|
Hi @Kevin-Jin, I tried this out on RCloud for a few days and it produces a couple of new errors. The first is that when I send an empty object from JavaScript it gets decoded as a list containing NULL, when it used to be an empty list. content.files
Object {}> foo$files
[[1]]
NULLThen this produces another error while processing the error, because a vector containing either NA or NULL gets transformed into just the value NULL, so decoding breaks here: var result = proto.json.call(this, resolver);
result.r_type = type;https://github.com/cscheid/rserve-js/blob/master/src/robj.js#L12 The subtleties of transcoding R and JavaScript data structures are... subtle. A transformation that seems really obvious for one application will not work for another application. Consider, for example this very long thread when Shiny switched JSON libraries. Fascinating stuff, but it makes it hard to transition from one scheme to another. |
Rserve.create().eval() converts NAs to null and Rserve.create().set() converts null to NA.
R's NA is still distinguished from R's NULL with this scheme because NA can only be found inside an array (possibly of length 1) while NULL can only be found outside an array.
Fix some inconsistent use of whitespace.