Open LTLA opened 6 months ago
FWIW I eventually debugged it by modifying the DelayedArray source to eliminate the try()
altogether in DelayedOp-class.R
. Just slapping res
onto the message wasn't enough, unfortunately, as I needed the traceback to figure out where it was coming from.
This is a true positive.
The unit tests in ScaledMatrix construct a ScaledMatrixSeed object for which extract_array()
doesn't work (non-compliant seed). To reproduce:
library(ScaledMatrix)
## Define the CrippledMatrix class and all the methods defined
## in ScaledMatrix/tests/testthat/test-mult.R by copy-pasting
## lines 9-59 from the file.
## Then:
cm <- new("CrippledMatrix", x=matrix(runif(50), ncol=5))
seed <- ScaledMatrixSeed(cm) # non-compliant seed!
SM <- DelayedArray(seed)
# Error in validObject(.Object) : invalid class “ScaledMatrix” object:
# the supplied seed must support extract_array()
This is a validity check that I added a few weeks ago in devel.
Trying to call extract_array()
directly on this seed indeed fails:
extract_array(seed, list(1:3, 1:2))
# Error in as.vector(data) :
# no method for coercing this S4 class to a vector
In release, SM <- DelayedArray(seed)
works but the object is broken e.g. as.matrix()
doesn't work on it:
as.matrix(SM)
# Error in h(simpleError(msg, call)) :
# error in evaluating the argument 'x' in selecting a method for function 'drop': no method for coercing this S4 class to a vector
or it can't be displayed:
show(SM)
# Error in h(simpleError(msg, call)) :
# error in evaluating the argument 'x' in selecting a method for function 'type': no method for coercing this S4 class to a vector
etc...
Purpose of this new validity check is to detect broken DelayedArray objects as early as possible i.e. at construction time.
Suggestions welcome to make the error message more informative.
H.
Yes, I'm fine with the motivation behind the check. It's just that the error message itself is not very informative.
It's unfortunate that we need to return a string in the validity method, because that loses the traceback.
I guess we could just not wrap it in a try
and just expose the traceback to the user. Currently the user doesn't even know who to ask to fix the problem, as the failing matrix could be arbitrarily deep in a tree of delayed operations.
A recent change in the DelayedArray stack (don't know exactly where) is causing ScaledMatrix tests to break on Bioc-devel with the rather unhelpful message:
This doesn't make any sense because
ScaledMatrix
(via its seed) does, in fact, implementextract_array()
. I assume there's atry
being done somewhere to test whetherextract_array()
works, which is catching and hiding the real error message.Session information
``` R Under development (unstable) (2023-11-10 r85507) Platform: x86_64-pc-linux-gnu Running under: Ubuntu 20.04.6 LTS Matrix products: default BLAS: /home/luna/Software/R/trunk/lib/libRblas.so LAPACK: /home/luna/Software/R/trunk/lib/libRlapack.so; LAPACK version 3.11.0 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 time zone: America/Los_Angeles tzcode source: system (glibc) attached base packages: [1] stats4 stats graphics grDevices utils datasets methods [8] base other attached packages: [1] DelayedArray_0.29.7 SparseArray_1.3.4 S4Arrays_1.3.4 [4] abind_1.4-5 IRanges_2.37.1 S4Vectors_0.41.3 [7] MatrixGenerics_1.15.0 matrixStats_1.2.0 BiocGenerics_0.49.1 [10] Matrix_1.6-5 ScaledMatrix_1.11.1 testthat_3.2.1 loaded via a namespace (and not attached): [1] vctrs_0.6.5 crayon_1.5.2 cli_3.6.2 rlang_1.1.3 [5] glue_1.7.0 fansi_1.0.6 brio_1.1.4 grid_4.4.0 [9] lifecycle_1.0.4 compiler_4.4.0 waldo_0.5.2 XVector_0.43.1 [13] rstudioapi_0.15.0 lattice_0.22-5 R6_2.5.1 utf8_1.2.4 [17] pillar_1.9.0 magrittr_2.0.3 tools_4.4.0 withr_3.0.0 [21] zlibbioc_1.49.0 desc_1.4.3 ```