grimbough / Rarr

A simple native R reader for Zarr Arrays
https://bioconductor.org/packages/Rarr/
MIT License
34 stars 5 forks source link

Missing `path` method for `ZarrArray` #11

Open Artur-man opened 6 days ago

Artur-man commented 6 days ago

It would be nice to allow zarr store path of a ZarrArray to be returned. I think this is already available in HDF5Array ?

> a <- array(runif(1e6), dim = c(100,100,100))
> tf1 <- tempfile(fileext = ".zarr")
> z1 <- Rarr::writeZarrArray(a, tf1, chunk_dim = c(50,50,50))
> DelayedArray::path(z1)
Error: unable to find an inherited method for function ‘path’ for signature ‘object = "ZarrArraySeed"’

Required here: https://github.com/HelenaLC/SpatialData.plot https://github.com/HelenaLC/SpatialData https://github.com/HelenaLC/SpatialData/pull/100

grimbough commented 3 days ago

You can now find a path() method in the devel version of Rarr:

suppressPackageStartupMessages(library(Rarr))
packageVersion('Rarr')
#> [1] '1.7.1'

a <- array(runif(1e6), dim = c(100,100,100))
tf1 <- tempfile(fileext = ".zarr")
z1 <- Rarr::writeZarrArray(a, tf1, chunk_dim = c(50,50,50))
path(z1)
#> [1] "/tmp/Rtmp3QU0Pu/file31b04175359a6.zarr/"

One thing I note is that it appends the trailing slash to the path. That makes sense when constructing the paths to chunks (better to do one paste operation and store, rather than one for each chunk access) but probably isn't what you'd expect to get from this. I haven't decided if I'll strip it or not, but maybe don't expect it to stay. Happy to received feedback on that. I see your pull request above uses normalizePath() which I think will also strip the trailing slash. Not sure what that'll do to an S3 URL if that was what was stored as the path.

wolfganghuber commented 3 days ago

FWIW, I asked chatgpt about trailing slash. Doesn’t seem conclusive but also provides no strong incentive to intervene. Thank you and kind regards Wolfgang

Il giorno 25.11.2024, alle ore 17:31, Mike Smith @.***> ha scritto:

You can now find a path() method in the devel version of Rarr: suppressPackageStartupMessages(library(Rarr)) packageVersion('Rarr')

> [1] '1.7.1'

a <- array(runif(1e6), dim = c(100,100,100)) tf1 <- tempfile(fileext = ".zarr") z1 <- Rarr::writeZarrArray(a, tf1, chunk_dim = c(50,50,50)) path(z1)

> [1] "/tmp/Rtmp3QU0Pu/file31b04175359a6.zarr/"

One thing I note is that it appends the trailing slash to the path. That makes sense when constructing the paths to chunks (better to do one paste operation and store, rather than one for each chunk access) but probably isn't what you'd expect to get from this. I haven't decided if I'll strip it or not, but maybe don't expect it to stay. Happy to received feedback on that. I see your pull request above uses normalizePath() which I think will also strip the trailing slash. Not sure what that'll do to an S3 URL if that was what was stored as the path. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

grimbough commented 3 days ago

Seems like the test for this is failing on Windows and Mac (https://github.com/grimbough/Rarr/actions/runs/12014461068/job/33490292420#step:9:657)

The test compares the output of path() against the actual path provided to writeZarrArray(). If they're not the same then that's not what I was expecting. It might just be that the test isn't well specified, but I should check what's returned on those platforms.