Bioconductor / DelayedArray

A unified framework for working transparently with on-disk and in-memory array-like datasets
https://bioconductor.org/packages/DelayedArray
24 stars 9 forks source link

sub-assignment for DelayedArrays doesn't follow base API #50

Closed sa-lee closed 5 years ago

sa-lee commented 5 years ago

Hi Hervé,

Just wondering if this is expected behaviour for subassignment on multi dimensional arrays?

suppressedPackageStartupMessages(library(DelayedArray))
seed <- array(NA_real_, dim = c(10, 2, 5))
f <- DelayedArray(seed)
# subassignment on regular array
seed[,,1] <- matrix(runif(20), ncol = 2)
# incompatible dimensions?
f[,,1] <- matrix(runif(20), ncol = 2)
#> Error in new_DelayedSubassign(x@seed, Nindex, value): dimensions of replacement value are incompatible with the number
#>   of array elements to replace

Created on 2019-09-19 by the reprex package (v0.3.0)

Session info ``` r devtools::session_info() #> ─ Session info ────────────────────────────────────────────────────────── #> setting value #> version R version 3.6.1 (2019-07-05) #> os macOS Mojave 10.14.6 #> system x86_64, darwin15.6.0 #> ui X11 #> language (EN) #> collate en_AU.UTF-8 #> ctype en_AU.UTF-8 #> tz Australia/Melbourne #> date 2019-09-19 #> #> ─ Packages ────────────────────────────────────────────────────────────── #> package * version date lib source #> assertthat 0.2.1 2019-03-21 [1] CRAN (R 3.6.0) #> backports 1.1.4 2019-04-10 [1] CRAN (R 3.6.0) #> BiocGenerics * 0.31.5 2019-07-03 [1] Bioconductor #> BiocParallel * 1.19.2 2019-08-07 [1] Bioconductor #> callr 3.3.1 2019-07-18 [1] CRAN (R 3.6.0) #> cli 1.1.0 2019-03-19 [1] CRAN (R 3.6.0) #> crayon 1.3.4 2017-09-16 [1] CRAN (R 3.6.0) #> DelayedArray * 0.11.4 2019-07-03 [1] Bioconductor #> desc 1.2.0 2018-05-01 [1] CRAN (R 3.6.0) #> devtools 2.2.0 2019-09-07 [1] CRAN (R 3.6.0) #> digest 0.6.20 2019-07-04 [1] CRAN (R 3.6.0) #> DT 0.8 2019-08-07 [1] CRAN (R 3.6.0) #> ellipsis 0.2.0.1 2019-07-02 [1] CRAN (R 3.6.0) #> evaluate 0.14 2019-05-28 [1] CRAN (R 3.6.0) #> fs 1.3.1 2019-05-06 [1] CRAN (R 3.6.0) #> glue 1.3.1 2019-03-12 [1] CRAN (R 3.6.0) #> highr 0.8 2019-03-20 [1] CRAN (R 3.6.0) #> htmltools 0.3.6 2017-04-28 [1] CRAN (R 3.6.0) #> htmlwidgets 1.3 2018-09-30 [1] CRAN (R 3.6.0) #> IRanges * 2.19.16 2019-09-13 [1] Bioconductor #> knitr 1.24 2019-08-08 [1] CRAN (R 3.6.0) #> lattice 0.20-38 2018-11-04 [1] CRAN (R 3.6.1) #> magrittr 1.5 2014-11-22 [1] CRAN (R 3.6.0) #> Matrix 1.2-17 2019-03-22 [1] CRAN (R 3.6.1) #> matrixStats * 0.55.0 2019-09-07 [1] CRAN (R 3.6.0) #> memoise 1.1.0 2017-04-21 [1] CRAN (R 3.6.0) #> pkgbuild 1.0.5 2019-08-26 [1] CRAN (R 3.6.0) #> pkgload 1.0.2 2018-10-29 [1] CRAN (R 3.6.0) #> prettyunits 1.0.2 2015-07-13 [1] CRAN (R 3.6.0) #> processx 3.4.1 2019-07-18 [1] CRAN (R 3.6.0) #> ps 1.3.0 2018-12-21 [1] CRAN (R 3.6.0) #> R6 2.4.0 2019-02-14 [1] CRAN (R 3.6.0) #> Rcpp 1.0.2 2019-07-25 [1] CRAN (R 3.6.1) #> remotes 2.1.0 2019-06-24 [1] CRAN (R 3.6.0) #> rlang 0.4.0 2019-06-25 [1] CRAN (R 3.6.0) #> rmarkdown 1.15 2019-08-21 [1] CRAN (R 3.6.0) #> rprojroot 1.3-2 2018-01-03 [1] CRAN (R 3.6.0) #> S4Vectors * 0.23.23 2019-09-13 [1] Bioconductor #> sessioninfo 1.1.1 2018-11-05 [1] CRAN (R 3.6.0) #> stringi 1.4.3 2019-03-12 [1] CRAN (R 3.6.0) #> stringr 1.4.0 2019-02-10 [1] CRAN (R 3.6.0) #> testthat 2.2.1 2019-07-25 [1] CRAN (R 3.6.0) #> usethis 1.5.1 2019-07-04 [1] CRAN (R 3.6.0) #> withr 2.1.2 2018-03-15 [1] CRAN (R 3.6.0) #> xfun 0.9 2019-08-21 [1] CRAN (R 3.6.0) #> yaml 2.2.0 2018-07-25 [1] CRAN (R 3.6.0) #> #> [1] /Library/Frameworks/R.framework/Versions/3.6/Resources/library ```
hpages commented 5 years ago

Hi Stuart,

The error message should have been clearer but the current implementation of delayed subassignment expects the right value to have the same number of dimensions as the array being modified. So for example in your case using array(runif(20), c(10, 2, 1)) on the right side would have worked.

Anyway, I made delayed subassignment a little bit more accommodating in DelayedArray 0.11.5. See commit 95d7e072d1e4aceacd46cebfeb5608b56f8e5529

H.

sa-lee commented 5 years ago

That makes sense! Thanks!