albertosantini / node-rio

Integration with Rserve, a TCP/IP server for R framework
https://github.com/albertosantini/node-conpa
MIT License
176 stars 35 forks source link

Roadmap 2.x #20

Closed albertosantini closed 8 years ago

albertosantini commented 9 years ago

Please, comment this issue about RFP (Request for Proposal) for rio 2.x.

Status - 15 nov 2015 - merged

(Don't forget any implementation needs to work in Linux and Windows box).

albertosantini commented 9 years ago

I am going to create a 2.x branch. @alexproca

ghost commented 9 years ago

:+1:

albertosantini commented 9 years ago

dev-2.x branch created.

Please, feel free to add PR there. :)

alexproca commented 9 years ago

:+1:

albertosantini commented 9 years ago

Added #22 about API evaluate refactoring.

albertosantini commented 9 years ago

Added #23 about source code refactoring.

albertosantini commented 9 years ago

I am going to create a dev-2.x tag on npm registry, allowing npm install rio@dev-2.x.

albertosantini commented 9 years ago

npm install rio@dev-2.x

I will update the dev tag quite often.

albertosantini commented 9 years ago

Recommended way to install dev releases: npm install git+https://github.com/albertosantini/node-rio.git#dev-2.x

I am going to delete dev-2.x (and dev-2.x.x) tag and 2.0.0-beta, 2.0.0-beta.1 releases, because npm doesn't support anymore -f option to overwrite a version and only npm admins actually can tweak dist-tags entries.

albertosantini commented 9 years ago

Just an update.

In the last few months I have been developing Argo, a trading platform to develop realtime strategies.

That project might be a good use case to test rio and finally to create change requests for 2.x. I did not forget rio. :)

albertosantini commented 9 years ago

Add debug package task.

satyashani commented 9 years ago

Proposals -

  1. The error passed to evaluate function is currently either a string or boolean true. I think it should be the Error("Error String") object which is more standard across nodejs. One expects to read err.message when err is not null.
  2. I cannot get rio to parse json response(I get SEXP type = 162). I think JSON should be supported. I've made these updates in my branch satyashani/node-rio@9c9eb8f. One test is added for json, and error checking tests are updated. All tests were passing when I checked.
albertosantini commented 9 years ago

@satyashani

satyashani commented 9 years ago

@albertosantini For error handling -

For type 162 I didn't find anything in docs , I don't work with R but have to connect to an instance. My proposal is based on what I found when I set enableDebug(true). The error was - Type 162 is currently not implemented

albertosantini commented 9 years ago

Hey @satyashani.

I think there is a misunderstanding. The binary protocol of Rserve, implemented in rio, can transfer, for instance, strings, but it doesn't exist a json type. Indeed you cannot find any documentation.

In rio examples I have been using RJSONIO package. Notice the final result it is a string.

This is R console transcript:

> require(RJSONIO)
Loading required package: RJSONIO
> jsonstr="{\"result\":{\"key1\":[1],\"key2\":[true]}}"
> fromJSON(jsonstr)
$result
$result$key1
[1] 1

$result$key2
[1] TRUE

> toJSON(fromJSON(jsonstr))
[1] "{\n \"result\": {\n \"key1\":      1,\n\"key2\": true \n} \n}"
> 

Now I execute the same commands using jsonlite package.

> require(jsonlite)
Loading required package: jsonlite

Attaching package: ‘jsonlite’

The following object is masked from ‘package:utils’:

    View

> jsonstr="{\"result\":{\"key1\":[1],\"key2\":[true]}}"
>  fromJSON(jsonstr)
$result
$result$key1
[1] 1

$result$key2
[1] TRUE

>  toJSON(fromJSON(jsonstr))
{"result":{"key1":[1],"key2":[true]}} 
> class(toJSON(fromJSON(jsonstr)))
[1] "json"
> 

You can notice the latest result is not a string, it is an R object, a class, and you need to serialize it. We cannot parse every object or class.

To pass json objects between R world and Node.js one, you can give a look at the examples ex3.js and ex3.R.

About the error handling I like this doc, confirming the best practice: https://www.joyent.com/developers/node/design/errors

callback(new Error('something bad happened'));

However I gave a look at mainstream project and there is not a consensus about the object type of err (string or Error object) in the callback. Generally speaking, I am not against it.

Other users may give their feedback about it.

satyashani commented 9 years ago

I'd rather ask my colleagues to use RJSONIO. I don't understand why S expression type is 162 when it is not implemented at all in Rserve. Thanks for your time on this.

albertosantini commented 9 years ago

rio 1.4.0 is a solid version.

I will merge dev-2.x branch in the master in a few weeks, creating 2.0.0 release.

I know some features have not been implemented, but, I think, it is worthy moving on and looking forward.

Thanks for the patience.

g2010a commented 9 years ago

Thank you for all the hard work; a feature that would be great to implement is encrypted authentication... R is single-threaded and not nice to other processes, so I've found it better to run it remotely on its own server -- you might eventually have to do that with Argo as well -- but plaintext authentication is a security problem.

albertosantini commented 8 years ago

At last merged. Polishing details in the next weeks and then publishing 2.0.0.

If anyone is interested to any 2.x point not implemented, please open a new issue. Contributing is welcome. :)