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

DelayedArray's log() doesn't respond to argument 'base' #92

Closed LTLA closed 3 years ago

LTLA commented 3 years ago
library(DelayedArray)
Z <- DelayedArray(rbind(123))
log(Z)
## <1 x 1> matrix of class DelayedMatrix and type "double":
##          [,1]
## [1,] 4.812184
log(Z, base=10)
## <1 x 1> matrix of class DelayedMatrix and type "double":
##          [,1]
## [1,] 4.812184

Interestingly enough, Matrix itself had a similar bug some time ago. Seems like base= is pretty easy to forget.

Session information ``` R Under development (unstable) (2021-01-24 r79876) Platform: x86_64-pc-linux-gnu (64-bit) Running under: Ubuntu 18.04.5 LTS Matrix products: default BLAS: /home/luna/Software/R/trunk/lib/libRblas.so LAPACK: /home/luna/Software/R/trunk/lib/libRlapack.so locale: [1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C [3] LC_TIME=en_US.UTF-8 LC_COLLATE=en_US.UTF-8 [5] LC_MONETARY=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8 [7] LC_PAPER=en_US.UTF-8 LC_NAME=C [9] LC_ADDRESS=C LC_TELEPHONE=C [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C attached base packages: [1] parallel stats4 stats graphics grDevices utils datasets [8] methods base other attached packages: [1] DelayedArray_0.17.11 IRanges_2.25.10 S4Vectors_0.29.17 [4] MatrixGenerics_1.3.1 matrixStats_0.58.0 BiocGenerics_0.37.4 [7] Matrix_1.3-2 loaded via a namespace (and not attached): [1] compiler_4.1.0 grid_4.1.0 lattice_0.20-41 ```
hpages commented 3 years ago

I guess that's because log(), like 34 other functions, belongs to the Math group generic, which takes only one argument:

> Math
groupGenericFunction for "Math" defined from package "base"

function (x) 
standardGeneric("Math")
<bytecode: 0x55a2f39a6b08>
<environment: 0x55a2f352cd10>
Methods may be defined for arguments: x
Use  showMethods(Math)  for currently available ones.

See ?methods::Math.

The DelayedArray methods for all the functions in that group are defined thru that generic, which is very convenient:

> selectMethod("Math", "DelayedArray")
Method Definition:

function (x) 
stash_DelayedUnaryIsoOpStack(x, function(a) match.fun(.Generic)(a))
<bytecode: 0x55a2fea34bc0>
<environment: namespace:DelayedArray>

Signatures:
        x             
target  "DelayedArray"
defined "DelayedArray"

This means that log() will need special treatment.

hpages commented 3 years ago

Fixed (commit f98fafea313f1a21f3447ce15f7b20f9ff07c639).

Of course, Rle and NumericList objects from the S4Vectors and IRanges packages also go thru the Math group generic to define all the Math methods and they have the same problem:

> log(Rle(1:5))
numeric-Rle of length 5 with 5 runs
  Lengths:        1        1        1        1        1
  Values : 0.000000 0.693147 1.098612 1.386294 1.609438

> log(Rle(1:5), base=10)
numeric-Rle of length 5 with 5 runs
  Lengths:        1        1        1        1        1
  Values : 0.000000 0.693147 1.098612 1.386294 1.609438

> log(NumericList(1:4, 10))
NumericList of length 2
[[1]] 0 0.693147180559945 1.09861228866811 1.38629436111989
[[2]] 2.30258509299405

> log(NumericList(1:4, 10), base=10)
NumericList of length 2
[[1]] 0 0.693147180559945 1.09861228866811 1.38629436111989
[[2]] 2.30258509299405

Darn, who knew that base::log() had a base argument and why would anybody need that when they can just do log(x)/log(base).