r-lib / rray

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

Should rray_bind() be able to accept 0 and negative integer values for .axis? #262

Closed mattwarkentin closed 3 weeks ago

mattwarkentin commented 4 years ago

Hi @DavisVaughan,

Wondering if it would be sensible to slightly extend the functionality of rray_bind() such that the .axis argument can accept values of 0 or negative integers in order to add dimensions to the "front" of the bound array.

Here is simple comparison between rray_bind() and abind(). Neither allow the full flexibility of adding dimensions on either side of the array, IMO. rray_bind() allows adding arbitrary number of dimensions to the "end" of the array, but not before; and abind() only allows adding a single additional dimension to the beginning or end.

library(tidyverse)
library(rray)
library(abind)

a <- array(1:100, dim = c(10, 10))
b <- array(101:200, dim = c(10, 10))

rray_bind(a, b, .axis = 1) %>% dim()
#> [1] 20 10
rray_bind(a, b, .axis = 2) %>% dim()
#> [1] 10 20
rray_bind(a, b, .axis = 3) %>% dim()
#> [1] 10 10  2
rray_bind(a, b, .axis = 10) %>% dim()
#>  [1] 10 10  1  1  1  1  1  1  1  2
rray_bind(a, b, .axis = 0) %>% dim()
#> Error: Invalid `.axis`.
#> The minimum value for `.axis` is 1.
#> The following `.axis` positions are incorrect: 1.

abind(a, b, along = 1) %>% dim()
#> [1] 20 10
abind(a, b, along = 2) %>% dim()
#> [1] 10 20
abind(a, b, along = 3) %>% dim()
#> [1] 10 10  2
abind(a, b, along = 10) %>% dim()
#> Error in abind(a, b, along = 10): along must be between 0 and 3
abind(a, b, along = 0) %>% dim()
#> [1]  2 10 10
abind(a, b, along = -1) %>% dim()
#> [1]  2 10 10

Created on 2020-08-12 by the reprex package (v0.3.0)

DavisVaughan commented 4 years ago

rray has been archived on CRAN, and will eventually require a complete rewrite without xtensor to really be viable in the R community. Unfortunately it will be quite a while before I can get around to that - but i will leave this issue open to consider when I do

mattwarkentin commented 4 years ago

I have another thought about another possible (very) minor feature request, I think it would be nice if the axis argument to rray_flip() accepted a vector of integers, and all of the flips are applied. Since the order of the flips doesn't affect the result, I think this would be a small change but a welcome one. Currently, if you want to flip along the x, y, and z axis of a 3D array you have to call the function three times over. Not a big deal, of course. The suggested change wouldn't break existing code, but would provide benefits to future code.

library(tidyverse)
library(rray)

a <- array(c(1:24), c(4, 3, 2))

a1 <-
  a %>% 
  rray_flip(1) %>% 
  rray_flip(2) %>% 
  rray_flip(3)

a2 <-
  a %>% 
  rray_flip(3) %>% 
  rray_flip(2) %>% 
  rray_flip(1)

rray_all_equal(a1, a2)
#> [1] TRUE

Created on 2020-08-18 by the reprex package (v0.3.0)