xtensor-stack / xtensor-r

R bindings for xtensor
BSD 3-Clause "New" or "Revised" License
86 stars 15 forks source link

Allow implicit conversion to `Rcpp::RObject` #114

Open DavisVaughan opened 5 years ago

DavisVaughan commented 5 years ago

Currently, something like this is allowed with no problems:

SEXP rray__exp_impl(const xt::rarray<rlogical>& x) {
  xt::rarray<double> res = xt::exp(x);
  return res;
}

The (potentially safer) approach would be to return an Rcpp::RObject like this:

Rcpp::RObject rray__exp_impl(const xt::rarray<rlogical>& x) {
  xt::rarray<double> res = xt::exp(x);
  return res;
}

But this gives me a "no viable conversion" error as it does not know how to convert to RObject from xt::rarray<double>. I can do something like this, but it is just a bit annoying:

Rcpp::RObject rray__exp_impl(const xt::rarray<rlogical>& x) {
  xt::rarray<double> res = xt::exp(x);
  return Rcpp::as<Rcpp::RObject>(res);
}

Do you know if its possible to add this implicit conversion?

PS: If you not aware, RObjects are wrappers around SEXPs that Rcpp added that help automatically manage the underlying storage properties (they are also the base class that all core Rcpp classes inherit from). So you don't have to call PROTECT() and UNPROTECT() like you would have to do on the raw R SEXP objects

wolfv commented 5 years ago

Hey, yes, it is definitely possible to add an implicit conversion like that. You'd just need to add a function like this:

rcontainer ... 
{
    operator Rcpp::RObject () {
         return Rcpp::as<Rcpp::RObject>(*this);
    }
}

However, I am unsure where to extend xtensor-r to support this. There is also a chance that Rcpp has an internal mechanism to do this (like they have for the SEXP casting, if you take a look at rcpp_extensions header). Maybe we can ask them?

DavisVaughan commented 5 years ago

I know that

as summarized nicely in the first code block here http://dirk.eddelbuettel.com/code/rcpp/Rcpp-extending.pdf

But I guess this is technically rarray<double> -> RObject which are 2 C++ types, so maybe it shouldn't go through that mechanism.

I think this last example (repeated below) might only be working because rcontainer inherits from PreserveStorage which I think has the capabilities to be cast like this because it "looks like" a SEXP. I'm not really sure.

Rcpp::RObject rray__exp_impl(const xt::rarray<rlogical>& x) {
  xt::rarray<double> res = xt::exp(x);
  return Rcpp::as<Rcpp::RObject>(res);
}

I can ask someone if you aren't sure either

wolfv commented 5 years ago

This works because you're explicitly asking for this conversion.

C++ allows one implicit conversion only. So implicitly you can convert to a SEXP like this: SEXP msxp = x; (where x is an rarray).

Now RObject has (most likely) a constructor from SEXP (which would require two conversions for implicit conversion) (rarray -> SEXP -> RObject). If you ask for it explicitly it would also work (RObject( (SEXP) x))

Does explicitly asking for RObject(x) work?

Btw. I think rarray ... use the exact same mechanisms as RObject so in theory at least it shouldn't matter that your object stays an rarray. Out of curiosity, what benefits do you get from the RObject cast?

DavisVaughan commented 5 years ago

Yea, both of these work:

// [[Rcpp::depends(xtensor)]]
// [[Rcpp::plugins(cpp14)]]

#include <xtensor-r/rarray.hpp>

// [[Rcpp::export(rng = false)]]
Rcpp::RObject rray__exp_explicit(const xt::rarray<rlogical>& x) {
  xt::rarray<double> res = xt::exp(x);
  return Rcpp::RObject((SEXP) res);
}

// [[Rcpp::export(rng = false)]]
Rcpp::RObject rray__exp_direct(const xt::rarray<rlogical>& x) {
  xt::rarray<double> res = xt::exp(x);
  return Rcpp::RObject(res);
}

As to why I'd do this, it's for the fact that RObject abstracts over the fact that I return a double, vs logical, vs integer, so the return type can be a bit more flexible.

A few functions, like the identity function +x in base R, return:

So I wanted to be able to do:

template <typename T>
xt::rarray<T> rray__identity_impl(const xt::rarray<T>& x) {
  return xt::operator+(x);
}

But that won't work for logicals because the return type should be different than the input type. So I can do something like:

template <typename T>
Rcpp::RObject rray__identity_impl(const xt::rarray<T>& x) {
  return xt::operator+(x);
}

Rcpp::RObject rray__identity_impl(const xt::rarray<rlogical>& x) {
  xt::rarray<int> res = xt::operator+(x);
  return Rcpp::as<Rcpp::RObject>(res);
}