Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Expose mode to NWBZarrIO #137

Open
3 tasks done
CodyCBakerPhD opened this issue Nov 7, 2023 · 6 comments · May be fixed by #229
Open
3 tasks done

[Feature]: Expose mode to NWBZarrIO #137

CodyCBakerPhD opened this issue Nov 7, 2023 · 6 comments · May be fixed by #229
Assignees
Labels
category: enhancement improvements of code or code behavior priority: medium non-critical problem and/or affecting only a small set of users

Comments

@CodyCBakerPhD
Copy link
Contributor

What would you like to see added to HDMF-ZARR?

Stems from catalystneuro/neuroconv#569 (comment)

The NWBHDF5IO has a .mode attribute publicly exposed to easily detect on an instantiated IO object what the mode was at time of instantiation (useful for determining if an in-memory NWBFile object was read from an existing file or created on the fly)

But the NWBZarrIO does not; though the same info can be accessed via the private ._ZarrIO__mode attribute

Is your feature request related to a problem?

from hdmf_zarr import NWBZarrIO

io = NWBZarrIO(path="../test2.nwb.zarr", mode="r")

io.mode

Traceback (most recent call last):

  Cell In[4], line 1
    io.mode

AttributeError: 'NWBZarrIO' object has no attribute 'mode'

What solution would you like?

Expose .mode attribute as public instead of _ZarrIO__mode

Do you have any interest in helping implement the feature?

No.

Code of Conduct

@oruebel
Copy link
Contributor

oruebel commented Nov 7, 2023

I think NWBZarrIO.file.mode should do the trick.

@CodyCBakerPhD
Copy link
Contributor Author

io.file.mode
Traceback (most recent call last):

  File ~\AppData\Roaming\Python\Python310\site-packages\zarr\hierarchy.py:520 in __getattr__
    return self.__getitem__(item)

  File ~\AppData\Roaming\Python\Python310\site-packages\zarr\hierarchy.py:500 in __getitem__
    raise KeyError(item)

KeyError: 'mode'

Also, more to the original issue as requested by @bendichter, he wanted a universal API consistency with NWBHDF5IO in this respect

@oruebel
Copy link
Contributor

oruebel commented Nov 7, 2023

If I remember correctly not all stores in Zarr actually support mode. The standard NestedDirectoryStore, e.g., I believe does not have a mode

@oruebel
Copy link
Contributor

oruebel commented Nov 7, 2023

Looking at https://zarr.readthedocs.io/en/stable/api/storage.html#zarr.storage.DirectoryStore (which is what Zarr uses by default), does not have a mode. The convenience functions zarr.convenience.open and other have a mode but I'm not sure right now where that is being stored (maybe, NWBZarrIO.file.store.mode)

@mavaylon1 mavaylon1 added category: enhancement improvements of code or code behavior priority: medium non-critical problem and/or affecting only a small set of users labels Apr 16, 2024
@oruebel
Copy link
Contributor

oruebel commented Nov 7, 2024

Since the Zarr DirectoryStore does not use a mode (which is the main store used in HDFM Zarr), we decided to close this issue. If a use case for this arises please reopen the issue.

@oruebel oruebel closed this as completed Nov 7, 2024
@oruebel
Copy link
Contributor

oruebel commented Nov 8, 2024

We can expose the mode used when calling ZarrIO, but that may not match exactly the mode that Zarr actually uses. E.g., we may use r when calling ZarrIO but the DirectoryStore in Zarr will still treat is as append. However, exposing the parameter that the user set on init seems still reasonable.

@oruebel oruebel reopened this Nov 8, 2024
@oruebel oruebel linked a pull request Nov 8, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: enhancement improvements of code or code behavior priority: medium non-critical problem and/or affecting only a small set of users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants