theislab / zellkonverter

Conversion between scRNA-seq objects
https://theislab.github.io/zellkonverter/
Other
145 stars 27 forks source link

dgRMatrix to dgCMatrix Conversion #55

Closed const-ae closed 3 years ago

const-ae commented 3 years ago

Hi Luke,

when I tried to load the https://data.caltech.edu/tindfiles/serve/f0d567c5-cea6-4a60-923e-e9fb4a4019e8/ dataset (from Svensson 2019 Nature Biotech Supplement), I get the following error:

>   se <- zellkonverter::readH5AD("../data/klein_2015.h5ad")
Error in as(mat, "dgCMatrix") : 
  no method or default for coercing "dgRMatrix" to "dgCMatrix"

The error occurs here, and I think it is related to the discussion in https://github.com/theislab/zellkonverter/issues/34 and the fix in https://github.com/theislab/zellkonverter/commit/732789e503f05ea05608f39bb7afd83114355a55

Unfortunately, it turns out that you cannot directly coerce a dgRMatrix to a dgCMatrix, but for some weird reason you have to coerce it to CsparseMatrix:

library(Matrix)
library(methods)
mat <- matrix(0, nrow = 10, ncol = 3)
mat[c(1, 14, 17)] <- 1:3
mat
#>       [,1] [,2] [,3]
#>  [1,]    1    0    0
#>  [2,]    0    0    0
#>  [3,]    0    0    0
#>  [4,]    0    2    0
#>  [5,]    0    0    0
#>  [6,]    0    0    0
#>  [7,]    0    3    0
#>  [8,]    0    0    0
#>  [9,]    0    0    0
#> [10,]    0    0    0
row_sp_mat <- as(mat, "dgRMatrix")
as(row_sp_mat, "dgCMatrix")
#> Error in as(row_sp_mat, "dgCMatrix"): no method or default for coercing "dgRMatrix" to "dgCMatrix"
as(row_sp_mat, "CsparseMatrix")
#> 10 x 3 sparse Matrix of class "dgCMatrix"
#>            
#>  [1,] 1 . .
#>  [2,] . . .
#>  [3,] . . .
#>  [4,] . 2 .
#>  [5,] . . .
#>  [6,] . . .
#>  [7,] . 3 .
#>  [8,] . . .
#>  [9,] . . .
#> [10,] . . .

Created on 2021-06-21 by the reprex package (v2.0.0)

Currently, I can work around the issue by defining

setAs("dgRMatrix", to = "dgCMatrix", function(from){
  as(as(from, "CsparseMatrix"), "dgCMatrix")
})

but that is, of course, inconvenient for the average user.

Best, Constantin

lazappi commented 3 years ago

Hi @const-ae

Thanks for the issue. This has been reported before but I haven't quite got around to fixing it. Will make it a priority now, should be pretty quick.

const-ae commented 3 years ago

Great. Thanks for the quick fix :)

lazappi commented 3 years ago

Should be fixed on release and devel now. Give Bioconductor a couple of days or install from GitHub if you want it right away.