Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

What if [[ relaxed the 1 element restriction #77

Closed
DavisVaughan opened this issue Apr 10, 2019 · 7 comments · Fixed by #90
Closed

What if [[ relaxed the 1 element restriction #77

DavisVaughan opened this issue Apr 10, 2019 · 7 comments · Fixed by #90
Labels
feature ✨ a feature request or enhancement

Comments

@DavisVaughan
Copy link
Member

If it used the full version of rray_yank() and rray_extract() then you could do

x[[is.na(x)]] <- 0

Which would be nice bc we don't allow that in [

@DavisVaughan
Copy link
Member Author

It's still type stable.

But most things don't let [[ return more than 1 element so that would be weird.

But you'd also be able to do x[[1:2]] to reproduce the base R behavior of x[1:2]

@DavisVaughan
Copy link
Member Author

The more I think about this the more I like it. [ Never reduces dimension. [[ Always does. But otherwise both can subset multiple elements

@DavisVaughan
Copy link
Member Author

Would obviously also allow

x[[1:2]] <- 0

@juangomezduaso
Copy link

I like the idea and hope you don`t have to discard it due to interactions with other functions, packages, etc. Considering rray in isolation, this design seems clear and sound.
There is still one form of subsetting to decide about: what R base calls numeric matrix indexing. To me that is kind of a mix between extract() and yank(): it has axial indexes , like extract(), but we dont want a cross produt, just an arbitrary collection (the rows of the matrix), like in yank().
Will it be implemented in rray? In another function? In any of the bracket functions?
I'd perhaps put it in [[ and [[<- , because these are asociated with yank() and extract(), and matrix indexing is a kind of mixture of these two.

This form of indexing would be almost innecesary if we had what I think is always usefull in an array package: the pair of functions to convert between indexes and positions in a given array (or dim). Havent looked for them in rray still, but supose they are somewhere

@DavisVaughan
Copy link
Member Author

DavisVaughan commented Apr 10, 2019

numeric matrix indexing

I had some internal conversation with some rstudio employees about this yesterday. The consensus was that it would probably be better to have a separate function that might help convert a matrix of this form to positions in the array, rather than support an obscure form of indexing.

xtensor has some helpers for this https://xtensor.readthedocs.io/en/latest/indices.html

@DavisVaughan
Copy link
Member Author

This seems to be as simple as flipping single = FALSE in [[ and [[<-. Maybe it would be worth supporting both with an optional argument? So you could do x[[c(1, 2), single = TRUE]] to get the single element enforcement so this example would error. And the default would be single = FALSE, which would allow x[[c(1, 2)]] and x[[c(1, 2)]] <- 0.

@juangomezduaso
Copy link

numeric matrix indexing

xtensor has some helpers

As in number of dimensions all arrays are really small, you can also afford the luxury of doing it in a "recursive R" oneliner if you will.

rray_i2p <- function(mat,dim){
  stopifnot(dim >= apply(mat,2,max))
  if(ncol(mat) == 1) as.vector(mat) else
  mat[,1]+dim[[1]]*(rray_i2p(mat[,2:ncol(mat),drop = FALSE], dim[-1])-1)
}

# Example
myIndexes <- matrix(c(3,1,3,2,1,2),ncol=3)
cbind(myIndexes,">", rray_i2p(myIndexes,c(3,5,2)))                   
#>      [,1] [,2] [,3] [,4] [,5]
#> [1,] "3"  "3"  "1"  ">"  "9" 
#> [2,] "1"  "2"  "2"  ">"  "19"

Created on 2019-04-11 by the reprex package (v0.2.1)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature ✨ a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants