Bioconductor / MatrixGenerics

S4 Generic Summary Statistic Functions that Operate on Matrix-Like Objects
https://bioconductor.org/packages/MatrixGenerics
12 stars 1 forks source link

Patch 1.12.1 not 100% backward compatible #34

Closed const-ae closed 1 year ago

const-ae commented 1 year ago

I just tried to make the released version of sparseMatrixStats compatible with the patched version of MatrixGenerics (v.1.12.1). I noticed that in some corner cases the behavior did change compared with the previous version 1.12.0

For example, colMeans with a center argument used to return a named output:

mat <- matrix(1:12, nrow = 4, ncol = 3)
colnames(mat) <- c("A", "B", "C")

center <- matrixStats::colMeans2(mat)
MatrixGenerics::colVars(mat, center = center, useNames = NA)
#>        A        B        C
#> 1.666667 1.666667 1.666667

packageVersion("MatrixGenerics")
#> [1] '1.12.0'
packageVersion("matrixStats")
#> [1] '0.62.0'

Created on 2023-06-09 with reprex v2.0.2

But now it doesn't have names anymore:

mat <- matrix(1:12, nrow = 4, ncol = 3)
colnames(mat) <- c("A", "B", "C")

center <- matrixStats::colMeans2(mat)
MatrixGenerics::colVars(mat, center = center, useNames = NA)
#> [1] 1.666667 1.666667 1.666667

packageVersion("MatrixGenerics")
#> [1] '1.12.1'
packageVersion("matrixStats")
#> [1] '1.0.0'

Created on 2023-06-09 with reprex v2.0.2


The same problem occurs for colWeightedMedians

hpages commented 1 year ago

Thanks Constantin. Good catch! So it looks like with matrixStats < 1.0.0, whether a function propagates the names or not (when called with useNames=NA) is not a simple yes or no answer. It's more like a "it depends" kind of answer!

I'll make the correction today for colVars/rowVars, colSds/rowSds, and colWeightedMedians/rowWeightedMedians.

hpages commented 1 year ago

Done in MatrixGenerics 1.12.2 (see 29f77e7197a38df960fd39488327842ac7a65c06)