xtensor-stack / xtensor-r

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

Add SEXP cast operator for roptional #101

Closed SylvainCorlay closed 5 years ago

SylvainCorlay commented 5 years ago

cc @DavisVaughan .

DavisVaughan commented 5 years ago

Oh wait does this have something to do with the Exporter that uses the now-renamed roptional_assembly https://github.com/QuantStack/xtensor-r/blob/master/include/xtensor-r/rcpp_extensions.hpp

DavisVaughan commented 5 years ago

Is it smart to also define wrap() methods for the rarray_optional and rtensor_optional types like how is done at the very bottom of rarray https://github.com/QuantStack/xtensor-r/blob/master/include/xtensor-r/rarray.hpp

I think (?) this gets us the automatic conversation from rarray_optional to SEXP which I would use a lot

DavisVaughan commented 5 years ago

In this PR the SEXP() operators were changed out for a different mechanism (relying on Rcpp storage policies maybe?). I don't actually know how SEXP() is currently working on rarray but we may want to implement this in the same way https://github.com/QuantStack/xtensor-r/pull/68

SylvainCorlay commented 5 years ago

In this PR the SEXP() operators were changed out for a different mechanism (relying on Rcpp storage policies maybe?). I don't actually know how SEXP() is currently working on rarray but we may want to implement this in the same way #68

I think that you are right. This should be done with the wrap method indeed.

I need to go give a class. Do you want to open the PR with that version? Closing this PR.

DavisVaughan commented 5 years ago

I'll give it a shot

SylvainCorlay commented 5 years ago

This is a good occasion to make the exporters for roptional_array and roptional_tensor.