Skip to content
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

Enable e.g. 'a[b > 3] = b[b > 3]' #352

Merged
merged 3 commits into from
Aug 29, 2015
Merged

Conversation

thirdwing
Copy link
Member

This for #345 .

It is just a combination of code from @kevinushey and @fabian-s .

I think converting to Vector is better than defining length() and get() for SubsetProxy.

@kevinushey
Copy link
Contributor

Thanks!

What happens when we attempt to assign non-conforming types, e.g. if x is a NumericVector and y is an IntegerVector? We should have test cases that ensure these throw or convert properly (seems like it could be hairy)

@thirdwing
Copy link
Member Author

#include <Rcpp.h>
using namespace Rcpp;

// [[Rcpp::export]]
IntegerVector subset_assign_subset(NumericVector x) {
    IntegerVector y(x.size());
    y[x > 3] = x[x > 3];
    return y;
}

/*

> Rcpp::sourceCpp("test.cpp")
> subset_assign_subset(1:6)
[1] 0 0 0 4 5 6
> x <- c(1.2, 2.3, 3.4, 4.5)
> subset_assign_subset(x)
[1] 0 0 3 4

*/

This can be added into unitTest, if necessary.

@kevinushey
Copy link
Contributor

It looks like type coercion is already properly handled so I think this is good to go. E.g. this gives a compiler error, as hoped:

#include <Rcpp.h>
using namespace Rcpp;

// [[Rcpp::export]]
IntegerVector subset_assign_subset(CharacterVector x) {
  IntegerVector y(x.size());
  y[x > 3] = x[x > 3];
  return y;
}

@eddelbuettel
Copy link
Member

Having a few unit tests would be nice. We have two so that is a start.

@eddelbuettel
Copy link
Member

Appreciate the extra tests! Merging now.

eddelbuettel added a commit that referenced this pull request Aug 29, 2015
Enable e.g. 'a[b > 3] = b[b > 3]'
@eddelbuettel eddelbuettel merged commit 3c72b6d into RcppCore:master Aug 29, 2015
@coatless coatless mentioned this pull request Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants