LTLA / scRNAseq

Clone of the Bioconductor repository for the scRNAseq package.
http://bioconductor.org/packages/devel/data/experiment/html/scRNAseq.html
24 stars 12 forks source link

test errors: Error in `h(simpleError(msg, call))`: error in evaluating the argument 'seed' in selecting a method for function 'DelayedArray': Error in h5checktype(). Argument not of class H5IdComponent. #55

Closed mr-c closed 3 months ago

mr-c commented 3 months ago

In Debian we are preparing to update the bioconductor packages to the 3.19 release.

The tests for scRNAseq are currently failing:

https://ci.debian.net/packages/r/r-bioc-scrnaseq/unstable/amd64/50041060/#L2218

142s  [ FAIL 3 | WARN 34 | SKIP 1 | PASS 51 ]
142s ══ Failed tests ════════════════════════════════════════════════════════════════
142s ── Error ('test-fetchDataset.R:16:5'): fetchDataset works as expected ──────────
142s Error in `h(simpleError(msg, call))`: error in evaluating the argument 'seed' in selecting a method for function 'DelayedArray': Error in h5checktype(). Argument not of class H5IdComponent.
[…]
142s ── Error ('test-fetchDataset.R:22:5'): fetchDataset realizes the reduced dimensions ──
142s Error in `h(simpleError(msg, call))`: error in evaluating the argument 'seed' in selecting a method for function 'DelayedArray': Error in h5checktype(). Argument not of class H5IdComponent.
[…]
142s ── Failure ('test-saveDataset.R:32:5'): saveDataset works as expected ──────────
142s `saveDataset(sce, tmp, meta)` did not throw an error.

Any hints? Is it benign, should we skip the affected tests?

The list of installed packages is at https://ci.debian.net/packages/r/r-bioc-scrnaseq/unstable/amd64/50041060/#S4

LTLA commented 3 months ago

No idea, I'm afraid; I've never seen this before, and it doesn't show up on the BioC build machines for 3.19.

From looking at the backtrace, the first two errors are issues with HDF5Array::H5SparseMatrixSeed. From digging further into the code, it seems that rhdf5::H5Dopen doesn't always throw an error when it fails to open a dataset, and returns FALSE instead. This suggests that either @grimbough should make it throw on failure (probably the most obvious solution), or @hpages should not attempt to H5Dclose a boolean in HDF5Array:::h5isdataset.

As for the last test error - it smells like there's something wrong with the V8 system library. saveDataset eventually calls down to jsonvalidate::json_validate(), which (IIUC) uses V8 to validate JSON files against the schema. That particular test checks that the validator actually detects invalid JSON files, so the lack of an error suggests that the V8 call is a no-op.

mr-c commented 3 months ago

@LTLA Thanks for the reply! I'll wait for the reply from your colleagues

As for the last test error - it smells like there's something wrong with the V8 system library. saveDataset eventually calls down to jsonvalidate::json_validate(), which (IIUC) uses V8 to validate JSON files against the schema. That particular test checks that the validator actually detects invalid JSON files, so the lack of an error suggests that the V8 call is a no-op.

Ah-ha, for this test failure I think that is the answer; we don't have jsonvalidate packaged for Debian yet, so I removed the call to json_validate from alabaster.base and gypsum. I'll patch this test out

hpages commented 3 months ago

Ah, I was not aware that rhdf5::H5Dopen() plus apparently a bunch of rhdf5::H5*open() and rhdf5::H5*create() functions can return FALSE instead of throwing an error when they fail. Thanks @LTLA! Looks like this has actually been the case since the introduction of the rhdf5 package in Bioconductor back in 2011. Not a big fan, especially since this was never documented.

I made some adjustments in HDF5Array to account for that: https://github.com/Bioconductor/HDF5Array/commit/fa5a1d3f75bb532296c7b962f62c707f4f9dbc3c This is in HDF5Array 1.32.1 (BioC 3.19) and 1.33.6 (BioC 3.20). Both versions should propagate to the Bioconductor package repositories in the next couple of days.

mr-c commented 3 months ago

Thank you @hpages ! I tested that patch and the errors in scRNAseq went away.