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

added optional RCPP_ARMADILLO_RETURN_COLVEC_AS_VECTOR and co. #151

Merged
merged 2 commits into from Aug 1, 2017

Conversation

Projects
None yet
2 participants
@sgsokol
Contributor

sgsokol commented Jul 29, 2017

I don't know how to add unit tests with RUnit so tested by hand with code snipets like the following:

legacy colvec

> sourceCpp(code="// [[Rcpp::depends(RcppArmadillo)]]\n#include <RcppArmadillo.h>\n// [[Rcpp::export]]\narma::vec add1(arma::vec x) {return(x+1);}")
> add1(1:2)
     [,1]
[1,]    2
[2,]    3

colvec tested with COLVEC

> sourceCpp(code="// [[Rcpp::depends(RcppArmadillo)]]\n#define RCPP_ARMADILLO_RETURN_COLVEC_AS_VECTOR\n#include <RcppArmadillo.h>\n// [[Rcpp::export]]\narma::vec add1v(arma::vec x) {return(x+1);}")
> add1v(1:2)
[1] 2 3

colvec tested with ANYVEC

> sourceCpp(code="// [[Rcpp::depends(RcppArmadillo)]]\n#define RCPP_ARMADILLO_RETURN_ANYVEC_AS_VECTOR\n#include <RcppArmadillo.h>\n// [[Rcpp::export]]\narma::vec add1v(arma::vec x) {return(x+1);}")
> add1v(1:2)
[1] 2 3

legacy rowvec

> sourceCpp(code="// [[Rcpp::depends(RcppArmadillo)]]\n#include <RcppArmadillo.h>\n// [[Rcpp::export]]\narma::rowvec add1r(arma::rowvec x) {return(x+1);}")
> add1r(1:2)
     [,1] [,2]
[1,]    2    3

rowvec tested with ROWVEC

> sourceCpp(code="// [[Rcpp::depends(RcppArmadillo)]]\n#define RCPP_ARMADILLO_RETURN_ROWVEC_AS_VECTOR\n#include <RcppArmadillo.h>\n// [[Rcpp::export]]\narma::rowvec add1rv(arma::rowvec x) {return(x+1);}")
> add1rv(1:2)
[1] 2 3

rowvec tested with ANYVEC

> sourceCpp(code="// [[Rcpp::depends(RcppArmadillo)]]\n#define RCPP_ARMADILLO_RETURN_ANYVEC_AS_VECTOR\n#include <RcppArmadillo.h>\n// [[Rcpp::export]]\narma::rowvec add1rv(arma::rowvec x) {return(x+1);}")
> add1rv(1:2)
[1] 2 3

Signed-off-by: Serguei Sokol

Serguei Sokol
added optional RCPP_ARMADILLO_RETURN_COLVEC_AS_VECTOR and co.
Signed-off-by: Serguei Sokol <sokol at insa-toulouse.fr>
@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Jul 29, 2017

Member

First off, thanks for getting this started!

Second, I edited your markdown. Level headers (one #) are 'too loud'; I made them level four. And I made your code snippets code snippets. See what I did there, and read up on Markdown and GitHub-flavoured Markdoiwn. It is worth it.

Third, this must have new unit tests. Adding them is trivial. Just look at the subdirectory and the runit.* files along with the supporting cpp/ directory and it should become clear. We may need several .cpp files with/without the defines.

Member

eddelbuettel commented Jul 29, 2017

First off, thanks for getting this started!

Second, I edited your markdown. Level headers (one #) are 'too loud'; I made them level four. And I made your code snippets code snippets. See what I did there, and read up on Markdown and GitHub-flavoured Markdoiwn. It is worth it.

Third, this must have new unit tests. Adding them is trivial. Just look at the subdirectory and the runit.* files along with the supporting cpp/ directory and it should become clear. We may need several .cpp files with/without the defines.

@eddelbuettel

This looks fine but is too minimal and bare.

We need unit tests.

We probably need a commented-out but documenting setting in RcppArmadilloConfig.h.

Serguei Sokol
added unit tests for RCPP_ARMADILLO_RETURN_COLVEC_AS_VECTOR and co.;
unit_test_setup() is modified to accept character vector as first argument;
@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Aug 1, 2017

Member

That looks good. I'll add ChangeLog and NEWS entries.

Member

eddelbuettel commented Aug 1, 2017

That looks good. I'll add ChangeLog and NEWS entries.

@eddelbuettel eddelbuettel merged commit 41f7930 into RcppCore:master Aug 1, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eddelbuettel eddelbuettel referenced this pull request Aug 1, 2017

Merged

vec as vector #154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment