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

vec as vector #154

Merged
merged 4 commits into from Aug 2, 2017

Conversation

Projects
None yet
2 participants
@sgsokol
Contributor

sgsokol commented Aug 1, 2017

Added unit tests.
To test all combinations of macros, we need several .cpp files:

  • one where no macro defined (armadillo.cpp);
  • another one where RCPP_ARMADILLO_RETURN_COLVEC_AS_VECTOR and RCPP_ARMADILLO_RETURN_ROWVEC_AS_VECTOR are defined (colrow_as_vec.cpp)
  • and the last one where RCPP_ARMADILLO_RETURN_ANYVEC_AS_VECTOR is defined (any_as_vec.cpp)

That's why the function unit_test_setup() from unit.test.R had to be modified to accept character vector as first argument. This modification is transparent for previous testing codes.

Serguei Sokol added some commits Jul 29, 2017

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Aug 1, 2017

Member

This overlaps with #151 which is not a good idea. Either expand the existing PR, or create a new one.

I can also just paste in the addition to the header.

Tests underway now, co-mingled with an upgrade to Armadillo 7.960.0. Both look good.

Member

eddelbuettel commented Aug 1, 2017

This overlaps with #151 which is not a good idea. Either expand the existing PR, or create a new one.

I can also just paste in the addition to the header.

Tests underway now, co-mingled with an upgrade to Armadillo 7.960.0. Both look good.

@sgsokol

This comment has been minimized.

Show comment
Hide comment
@sgsokol

sgsokol Aug 1, 2017

Contributor

Sorry for overlapping PRs. Now I have found instructions written in tiny characters "Add more commits by pushing to the master branch on sgsokol/RcppArmadillo". Hoping it will work as expected next time.

Contributor

sgsokol commented Aug 1, 2017

Sorry for overlapping PRs. Now I have found instructions written in tiny characters "Add more commits by pushing to the master branch on sgsokol/RcppArmadillo". Hoping it will work as expected next time.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Aug 1, 2017

Member

It happens. Git can be complicated. I can probably just squash this PR on top; it will be smart enough to not reapply the two already-merged commits.

Member

eddelbuettel commented Aug 1, 2017

It happens. Git can be complicated. I can probably just squash this PR on top; it will be smart enough to not reapply the two already-merged commits.

@eddelbuettel eddelbuettel merged commit a3aa9db into RcppCore:master Aug 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment