r-lib / rray

Simple Arrays
https://rray.r-lib.org
GNU General Public License v3.0
130 stars 12 forks source link

rray removes custom classes #247

Open paul-buerkner opened 4 years ago

paul-buerkner commented 4 years ago

We are currently in the process of writing a new package to efficiently handle posterior samples as return by MCMC samplers of Bayesian models and we consider using rray for matrix and array representations. However, for easy S3 method dispatch, we need to have custom classes on top of rray's classes that simply indicate that the rray is in the right format (without having to check again every time). Unfortunately, rray removes custom classes, while base R array does not. Here is a reproducible example:

A <- array(1:27, dim = c(3, 3, 3))
class(A) <- c("draws_array", class(A))
class(A)
> [1] "draws_array" "array" 
class(A+A)
> [1] "draws_array" "array" 

B <- rray::as_rray(array(1:27, dim = c(3, 3, 3)))
class(B) <- c("draws_array", class(B))
class(B)
> [1] "draws_array"    "vctrs_rray_int" "vctrs_rray"     "vctrs_vctr"
class(B+B)
> [1] "vctrs_rray_int" "vctrs_rray"     "vctrs_vctr"

Is there a way to preserve custom classes for all vectorized (pointwise) operations without having to redefine every numerical function rray provides for its objects?

DavisVaughan commented 4 years ago

There is, but I haven't exported it yet (it isn't fully tested). If you look at rray_add(), which is what + eventually calls, you'll see vec_ptype_container2() and vec_cast_container() called at the end. These are double dispatching generic functions which restore the "container type" on top of the raw array that is returned from the C code.

In a dev version I had exported these, but decided against it before release because I am not sure if this is a good long term solution for extensibility. The ideas are still being developed in vctrs, and I didn't want to provide something that we would have to immediately break. vctrs doesn't currently have the "container type" idea sorted out yet (see https://github.com/r-lib/vctrs/issues/211).

I'm not currently working on rray, so it would be awhile before I could get back around to this. If you want to take a shot at reexporting vec_cast_container and vec_ptype_container2 in a fork, then defining methods for them in your package for your draws_array class, then that might work. If it works smoothly, I'd be willing to review a PR for this.

If you aren't familiar with the double dispatch idea yet, see this vignette: https://vctrs.r-lib.org/articles/s3-vector.html#double-dispatch

Here is the commit where I un-exported the functions. You'd have to add the roxygen tags back (also note that there was a name change after this type -> ptype), and maybe change some of the internal documentation that I actually left in. https://github.com/r-lib/rray/commit/97c8f5fb0f5e768046fe44acfb4215909333a4e3

DavisVaughan commented 4 years ago

Do be aware that due to some xtensor-r bugs, I was not able to get missing values implemented in this first version šŸ˜¢ https://github.com/r-lib/rray/issues/21

paul-buerkner commented 4 years ago

Thank you for this super helpful answer! I will take a look at the non-exported solution you provided. At least for the time being, it may be safer for our purposes to use base R arrays (with drop defaulting to FALSE) and switch to rrays once there is an exported way preserving custom classes in rray.