Bioconductor / MatrixGenerics

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

Update documentation of useNames in BioC 3.18? #35

Closed PeteHaitch closed 11 months ago

PeteHaitch commented 11 months ago

For BioC 3.18, should we change the documentation of useNames from https://github.com/Bioconductor/MatrixGenerics/blob/09cb5cb17e28eaed15a917624170d19f6ff9d12e/man-roxygen/useNamesParameter.R#L1

to mimic/copy that of matrixStats:

If FALSE (default), no naming support is done. Else if `TRUE`, names attributes of result are set.)

Or do we need another release cycle where useNames = NA is supported?

hpages commented 11 months ago

Everybody has their own interpretation of what useNames=NA should do:

m <- matrix(1:12, nrow=4, dimnames=list(LETTERS[1:4], NULL))

library(MatrixGenerics)
library(Matrix)
library(sparseMatrixStats)
library(SparseArray)
library(DelayedArray)

matrixStats:

rowVars(m, useNames=NA)
# [1] 16 16 16 16
# Warning message:
#  useNames = NA is deprecated. Instead, specify either useNames = TRUE or useNames = TRUE. 

sparseMatrixStats:

rowVars(as(m, "dgCMatrix"), useNames=NA)
# [1] 16 16 16 16

SparseArray:

rowVars(as(m, "SparseArray"), useNames=NA)
#  A  B  C  D 
# 16 16 16 16 

DelayedArray:

rowVars(DelayedArray(m), useNames=NA)
# Error in BLOCK_rowVars(x, na.rm = na.rm, center = center, useNames = useNames) : 
#   'useNames' must be TRUE or FALSE

(And I'm guilty for not being consistent across my own packages.)

Anyways, rather than trying to document chaos, we should probably remove any reference to useNames=NA in MatrixGenerics's documentation. It's no longer officially supported so its meaning is undefined. :stuck_out_tongue_winking_eye:

H.

PeteHaitch commented 11 months ago

I'll try to prepare a PR next week