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

Rethink trailing comma behavior with subsetting #252

Open
DavisVaughan opened this issue Nov 26, 2019 · 0 comments
Open

Rethink trailing comma behavior with subsetting #252

DavisVaughan opened this issue Nov 26, 2019 · 0 comments
Labels
breaking change ☠️ API change likely to affect existing code bug 🐛 an unexpected problem or unintended behavior

Comments

@DavisVaughan
Copy link
Member

The terminology for what we do with trailing commas is incorrect. We don't "drop trailing commas". It is more appropriate for us to "expand unspecified trailing indices"

This requires a very small change in the code, changing to .ignore_empty = "none" here. But requires changes to the documentation and more tests.

rray/R/subset.R

Line 179 in 525539d

indexer <- dots_list(..., .preserve_empty = TRUE, .ignore_empty = "trailing")

This is what we should get:

library(rray)

x <- rray(1:8, c(2,2,2))

dim(x)
#> [1] 2 2 2

# max number of commas allowed is dim-1
x[,,1]
#> <rray<int>[,2,1][2]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> [2,]    2    4

# missing dimensions are always "expanded out"

# equivalent - expanded to `x[,,]`
x[]
#> <rray<int>[,2,2][2]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> [2,]    2    4
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7
#> [2,]    6    8
x[,]
#> <rray<int>[,2,2][2]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> [2,]    2    4
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7
#> [2,]    6    8
x[,,]
#> <rray<int>[,2,2][2]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> [2,]    2    4
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7
#> [2,]    6    8

# error - max number of commas is `3 - 1 = 2`
x[,,,]
#> Error: The dimensionality of `x` is 3. Cannot subset into dimension 4.

# equivalent - expanded to `x[1,,]`
x[1]
#> <rray<int>[,2,2][1]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7
x[1,]
#> <rray<int>[,2,2][1]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7
x[1,,]
#> <rray<int>[,2,2][1]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7

# equivalent - expanded to `x[,1,]`
x[,1]
#> <rray<int>[,1,2][2]>
#> , , 1
#> 
#>      [,1]
#> [1,]    1
#> [2,]    2
#> 
#> , , 2
#> 
#>      [,1]
#> [1,]    5
#> [2,]    6
x[,1,]
#> <rray<int>[,1,2][2]>
#> , , 1
#> 
#>      [,1]
#> [1,]    1
#> [2,]    2
#> 
#> , , 2
#> 
#>      [,1]
#> [1,]    5
#> [2,]    6

# with padding - "padding applied before expansion"
# (number of dims specified = 1, pad twice)
x[pad(), 1] # == x[,,1]
#> <rray<int>[,2,1][2]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> [2,]    2    4

# (number of dims specified = 2, pad once)
x[pad(), 1, ] # == x[, 1,]
#> <rray<int>[,1,2][2]>
#> , , 1
#> 
#>      [,1]
#> [1,]    1
#> [2,]    2
#> 
#> , , 2
#> 
#>      [,1]
#> [1,]    5
#> [2,]    6

# (number of dims specified = 3, no padding)
x[pad(), 1, ,] # == x[1, ,]
#> <rray<int>[,2,2][1]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3
#> 
#> , , 2
#> 
#>      [,1] [,2]
#> [1,]    5    7

# (number of dims specified = 2, pad once)
x[1, pad(), 1]
#> <rray<int>[,2,1][1]>
#> , , 1
#> 
#>      [,1] [,2]
#> [1,]    1    3

Created on 2019-11-26 by the reprex package (v0.3.0.9000)

@DavisVaughan DavisVaughan added breaking change ☠️ API change likely to affect existing code bug 🐛 an unexpected problem or unintended behavior labels Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ☠️ API change likely to affect existing code bug 🐛 an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

1 participant