search

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

Integer and integer array #18

Closed kmadathil closed 9 years ago

kmadathil commented 9 years ago

I've added code to handle integer/integer array (returned as type SEXP 32 from Rserve). tests/basic-tests.js has been enhanced to run tests for integers/integer arrays.

albertosantini commented 9 years ago

Nice. Thanks a lot. I will merge it in the next hours (and publish a new release).

albertosantini commented 9 years ago

Please, give a look at Travis CI build. Thanks.

kmadathil commented 9 years ago

Sorry if this is a dumb question, but does the travis-ci environment have R and rserve? I see all the other tests have if (!err) so they will pass even without R. I had removed that in my test.

npm test passes on my machine.

♢ Basic tests string array test ✓ get the string array boolean test ✓ get the boolean value string test ✓ get the string boolean array test ✓ get the boolean array double number test ✓ get the double number utf8 string test ✓ get the utf8 string integer array test ✓ get the integer array double number array test ✓ get the double number array integer test ✓ get the integer ♢ Login tests login ok test ✓ eval with login ok login ko test ✓ eval with login ko ♢ Eval errors tests require test ✓ missing package unknown test ✓ getting unknown syntax test ✓ getting syntax error library test ✓ missing package ♢ Image tests first image test ✓ get image ✓ OK » 16 honored (0.462s)

vows:all done

Done, without errors. karthik@KarthikMadathil:~/public_html/node-rio$ git log commit 01572c5ae4122ddbc857187770ef1315866a04fe Author: Karthik Madathil karthik@tenpointacademy.com Date: Thu Nov 13 20:41:29 2014 +0530

test for integer and integer array

On Thu Nov 13 2014 at 8:55:59 PM Alberto Santini notifications@github.com wrote:

Please, give a look at Travis CI build. Thanks.

— Reply to this email directly or view it on GitHub https://github.com/albertosantini/node-rio/pull/18#issuecomment-62906413 .

albertosantini commented 9 years ago

I think the point is the dump file test/dump/integer-test.bin.

Indeed there is not installed R and Rserve and that file is the dump of the conversation between rio and Rserve.

albertosantini commented 9 years ago

I think I should write two lines about that in the contrib doc.

Don't worry. I will fix it.

kmadathil commented 9 years ago

Thanks.

On Thu Nov 13 2014 at 9:11:29 PM Alberto Santini notifications@github.com wrote:

I think I should write two lines about that in the contrib doc.

Don't worry. I will fix it.

— Reply to this email directly or view it on GitHub https://github.com/albertosantini/node-rio/pull/18#issuecomment-62910252 .

kmadathil commented 9 years ago

Thanks, from your description of enableRecordMode, it is clear what I should've done for the test. Will keep that in mind next time.

On Fri Nov 14 2014 at 11:15:40 PM Alberto Santini notifications@github.com wrote:

Merged #18 https://github.com/albertosantini/node-rio/pull/18.

— Reply to this email directly or view it on GitHub https://github.com/albertosantini/node-rio/pull/18#event-193488086.

albertosantini commented 9 years ago

No problem. There was also an issue with the err. I fixed it and, now, if the file doesn't exist it is arisen an exception.

Thanks a lot for the contribution.