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

WIP: Ensure that zarr.ZipStores are closed #4395

Closed
wants to merge 1 commit into from

Conversation

hmaarrfk
Copy link
Contributor

@hmaarrfk hmaarrfk commented Aug 31, 2020

ZipStores aren't always closed making it hard to use them as fluidly as regular zarr stores.

  • Closes #xxxx
  • Tests added
  • Passes isort . && black . && mypy . && flake8 # master doesn't pass black
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

@hmaarrfk hmaarrfk changed the title Ensure that zarr.ZipStores are closed WIP: Ensure that zarr.ZipStores are closed Aug 31, 2020
@hmaarrfk
Copy link
Contributor Author

I'm not too sure about this anymore.

with the way the test is written now, it is unclear to me if the store should be closed afterward.

I'm also unsure of how to deal with the case where the user passed it a ZipStore instead of a string. Will have to keep thinking.

@jhamman
Copy link
Member

jhamman commented Jan 31, 2023

@hmaarrfk - wondering what your current thoughts on this issue are? Perhaps this can be handled better upstream in zarr-python? Should we close this?

@hmaarrfk
Copy link
Contributor Author

I'm not sure. I decided not to use zarr (not now) so i lost interest sorry.

@hmaarrfk hmaarrfk closed this Jan 31, 2023
@hmaarrfk
Copy link
Contributor Author

Ultimately, I'm not sure how you want to manage resources. This zarr store could be considered a resource and thus, may have an owner. Or maybe zarr should close itself upon garbage cleanup.

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

Successfully merging this pull request may close these issues.

3 participants