Add TileDB - #135
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
==========================================
+ Coverage 90.48% 91.66% +1.17%
==========================================
Files 6 6
Lines 1870 2542 +672
==========================================
+ Hits 1692 2330 +638
- Misses 178 212 +34 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ressions The new TileDB test file never ran in CI: TileDBArray is Bioconductor-only, so devtools::install_deps() could not resolve it and skip_if_no_tiledb() skipped all 748 lines with a green build. Install tiledb/jsonlite via apt (both in r2u), TileDBArray via BiocManager, and add a verify step so a missing stack fails loudly instead of skipping silently. Bug fixes: * .modelarray_backend() returned a named scalar for merged objects, because mergeModelArrays() keys the backend vector by scalar name. Every caller compares with identical(., "tiledb"), which a surviving name makes FALSE, so on any merged object the TileDB row cache silently never engaged and elementMetadata() fell through to rhdf5::h5read() on a TileDB directory. * writeResults()'s HDF5 branch kept the old deparsed-class check while the TileDB path used the new .prepare_results_for_storage(). The two disagree on numeric/integer columns carrying an extra class attribute: HDF5 factorized them, replacing the values with LUT indices. Both backends now share one implementation. * .read_scalar_row()'s non-contiguous fallback used `[[` on a named vector, which throws "subscript out of bounds" for an absent name rather than returning NULL, making the fall-through to a direct storage read dead code. * write_results_compression_level was validated only on the TileDB path; hoisted above the backend branch to match .init_scalar_stream_writer(). * Backend auto-detection claimed any existing directory as a TileDB store, and the streaming writers unlink the group they replace. Now requires a .tdb suffix or a directory that actually contains scalars/ or results/. * .write_hdf5_attribute() used create_attr(), which errors when the attribute already exists, unlike the h5writeAttribute() it replaced. Output regressions: * show() had dropped per-scalar element counts. Keep the new layout, which the vignettes document, and restore the counts as "FD (182581 elements)". * print.h5summary() emitted "H5 file:/path"; the vignettes document the space after the colon. Tests: cross-backend writeResults agreement, overwrite = FALSE on TileDB, a multi-scalar store driving a scalar predictor, chunked reads verified against unchunked with an out-of-order subset and n_cores = 2, mergeModelArrays across mixed backends, and auto-detection boundaries. Replaced the self-transposing reader in the HDF5/TileDB equivalence test with an orientation assertion, which was masking the bug class most likely in a new storage backend. Also drop the committed generated README.html (it was shipping in the tarball), regenerate man/ so h5summary.Rd becomes ModelArraySummary.Rd, de-duplicate a test helper, and reindent three added lines so the change is lint-neutral.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.