zarr-developers / zarr-python

An implementation of chunked, compressed, N-dimensional arrays for Python.

https://zarr.readthedocs.io

MIT License

1.46k stars 274 forks source link

LocalStore.list_prefix adds directory prefix to all paths #2190

Open dcherian opened 1 week ago

dcherian commented 1 week ago

Zarr version

Numcodecs version

Python Version

Operating System

Installation

Description

list_prefix("") for a LocalStore seems inconsistent with the other stores. Discovered in #2189

'/private/var/folders/sc/hsdthq2x7mnbfvwnk7rp4xmr0000gn/T/pytest-of-deepak/pytest-854/test_zarr_hierarchy_local_0/zarr.json'

Also: https://github.com/zarr-developers/zarr-python/blob/b1ecdd544823404faf37e3111e868261178aa3e5/tests/v3/test_store/test_local.py#L39-L40 LOL

Steps to reproduce

Additional output

No response

zoj613 commented 1 week ago

In my very inexperienced eyes, this looks like the cause: https://github.com/zarr-developers/zarr-python/blob/b1ecdd544823404faf37e3111e868261178aa3e5/src/zarr/store/local.py#L204-L211

The root directory path is not stripped away and returned as is (line 206). I am not sure why the code appears to be duplicated in that function? Maybe someone who was implementing the correct code (lines 208 - 211) forgot to delete the previous block?

jhamman commented 6 days ago

I think @d-v-b is working on a related set of fixes here: https://github.com/zarr-developers/zarr-python/pull/2064

d-v-b commented 6 days ago

yep, and I can split the fixes out of #2064 into their own PR

d-v-b commented 4 days ago

2064 is in, so I'm curious to see if the problem you observed is still around.

https://github.com/zarr-developers/zarr-python/blob/b1ecdd544823404faf37e3111e868261178aa3e5/tests/v3/test_store/test_local.py#L39-L40

LOL indeed, this bit me when working on the PR because I had no idea why my changes to the localstore implementation were not making tests fail :)