Skip to content

Read zarr v2 NWB files with zarr-python 3 - #4875

Open
h-mayorquin wants to merge 2 commits into
SpikeInterface:mainfrom
h-mayorquin:fix_nwb_zarr_3
Open

h-mayorquin wants to merge 2 commits into
SpikeInterface:mainfrom
h-mayorquin:fix_nwb_zarr_3

Conversation

@h-mayorquin

Copy link
Copy Markdown
Contributor

This PR ensures that we can read NWB zarr files in the zarr v2 format, written by hdmf-zarr<0.14, as we move forward to zarr-python 3. The problem currently is that zarr-python 3 cannot parse some of the arrays in these old files (variable-length string columns with an integer fill value, for example), so opening them fails.

What this PR does is use the backward compatibility of the pynwb reader to handle the old files: hdmf-zarr reads them with NWBZarrV2IO. That way, our in-house zarr reader remains simpler, since it only needs to support zarr v3. The format is detected from the root metadata file (zarr.json or .zgroup) without opening the folder, both for local files and for files streamed with stream_mode="zarr", and v2 files are routed to pynwb with a warning, since it can be slower. NWBZarrV2IO is called with allow_pickle=True because hdmf-zarr<0.14 pickled object references, such as the group column of the electrodes table, so most of these files cannot be read without it. Since hdmf-zarr 0.14 requires Python 3.12, reading v2 files with zarr-python 3 needs Python 3.12 or newer; on Python 3.11 the in-house reader still reads v3 files.

This is a different approach from #4260, which adds zarr v3 support and still reads v2 files with our in-house reader, working around zarr-python 3: it lists the store directly and skips the children that cannot be parsed. With this PR, those workarounds are no longer needed.

I went with the simplest way to unblock reading these files. If users of the old datasets need faster reading, we can add backward support to the in-house reader later, backed by a benchmark that shows the gain is worth the extra code.

Route zarr v2 NWB files, local or streamed with stream_mode="zarr", to pynwb, where
hdmf-zarr's NWBZarrV2IO keeps the backward compatibility, so the in-house reader only
needs to read zarr v3.
With zarr-python 2 the in-house reader reads zarr v2 files, and the pynwb streaming
path passes an FSStore that hdmf-zarr<0.14 rejects. Raise a clear error when hdmf-zarr
is missing, and stop skipping the zarr streaming test with zarr-python 3.
@alejoe91

Copy link
Copy Markdown
Member

Thansk @h-mayorquin, that sounds good!

Is it ok to first merge #4260 and then remove the added logic to use this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants