single-cell-data / TileDB-SOMA

Python and R SOMA APIs using TileDB’s cloud-native format. Ideal for single-cell data at any scale.
https://tiledbsoma.readthedocs.io
MIT License
90 stars 25 forks source link

[r] Fix `DenseNDArray` write after create #2970

Closed johnkerl closed 1 month ago

johnkerl commented 1 month ago

Issue and/or context: Found while debugging an issue with #2950 for issue #2407 / [sc-51048].

Changes:

Make it possible to write to a DenseNDArray using an open-for-write after the array is created.

Notes for Reviewer:

Note that the create methods return an array opened for write.

For SparseNDArray and DenseNDArray you can do:

For SparseNDArray, you can do:

For DenseNDArray, this does not work before this PR. The error message is

Error: soma_data must be a DataType, not NULL

You can reproduce this by including in your checkout the mod to apis/r/tests/testthat/test-SOMADenseNDArray.R on this PR but excluding the mod to apis/r/R/SOMADenseNDArray.R on this PR, then testing, e.g.:

> devtools::test_active_file(file="tests/testthat/test-SOMADenseNDArray.R")
TileDB-SOMA R package 1.13.99.6 with TileDB Embedded 2.25.0 on macOS Sonoma 14.6.1.
See https://github.com/single-cell-data for more information about the SOMA project.

══ Testing test-SOMADenseNDArray.R ═════════════════════════════════════════════════════════════════════════════════════
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 39 ]

── Error (test-SOMADenseNDArray.R:18:3): SOMADenseNDArray creation ─────────────
Error: soma_data must be a DataType, not NULL
Backtrace:
    ▆
 1. └─ndarray$write(mat) at test-SOMADenseNDArray.R:18:3
 2.   ├─arrow::schema(arrow::field("soma_data", private$.type)) at r/R/SOMADenseNDArray.R:138:7
 3.   │ └─rlang::list2(...)
 4.   └─arrow::field("soma_data", private$.type)
 5.     └─arrow:::as_type(type, name)
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 39 ]

Including this PR's bugfix on apis/r/tests/testthat/test-SOMADenseNDArray.R, one sees

> devtools::test_active_file(file="tests/testthat/test-SOMADenseNDArray.R")
TileDB-SOMA R package 1.13.99.6 with TileDB Embedded 2.25.0 on macOS Sonoma 14.6.1.
See https://github.com/single-cell-data for more information about the SOMA project.

══ Testing test-SOMADenseNDArray.R ═════════════════════════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 52 ] Done!

The root cause is that private$.type is set at create time but not recovered on the open-for-write. This bug was fixed here for SOMASparseNDArray on PR #2755; here we apply that same fix to DenseNDArray.

johnkerl commented 1 month ago

Thanks @mojaveazure !