Skip to content
This repository has been archived by the owner on Oct 24, 2024. It is now read-only.

Do not call __exit__ on Zarr store when opening #90

Merged
merged 2 commits into from
May 18, 2022

Conversation

thewtex
Copy link
Contributor

@thewtex thewtex commented May 16, 2022

The with context when opening the zarr group with result in calling
exit on the store when the function completes. This calls .close()
on ZipStore's, which results in errors:

ValueError: Attempt to use ZIP archive that was already closed

@thewtex
Copy link
Contributor Author

thewtex commented May 16, 2022

@TomNicholas please take a look

Tests were not passing for me locally prior to this fix. Edit: This was a problem with my zarr install -- all tests pass.

The `with` context when opening the zarr group with result in calling
__exit__ on the store when the function completes. This calls `.close()`
on ZipStore's, which results in errors:

```
ValueError: Attempt to use ZIP archive that was already closed
```
@thewtex
Copy link
Contributor Author

thewtex commented May 16, 2022

Test added

except zarr.errors.PathNotFoundError:
subgroup_ds = Dataset()

# TODO refactor to use __setitem__ once creation of new nodes by assigning Dataset works again
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomNicholas - is this still an issue we need to work around?

Copy link

@jhamman jhamman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I'm curious if we still need the workaround in io.py:99 but its not a core part of the change here so I'm fine merging as is.

@TomNicholas
Copy link
Member

Thanks @jhamman.

@thewtex said he needs this fix so I'm going to merge this and we can discuss that workaround in #92.

@thewtex
Copy link
Contributor Author

thewtex commented May 23, 2022

@jhamman @TomNicholas thanks!!

@thewtex
Copy link
Contributor Author

thewtex commented May 23, 2022

@TomNicholas any plans to cut a 0.0.6 release?

@TomNicholas
Copy link
Member

@TomNicholas any plans to cut a 0.0.6 release?

@thewtex I just released 0.0.6 just now!

flamingbear pushed a commit to flamingbear/rewritten-datatree that referenced this pull request Jan 19, 2024
…ee#90

* Do not call __exit__ on Zarr store when opening

The `with` context when opening the zarr group with result in calling
__exit__ on the store when the function completes. This calls `.close()`
on ZipStore's, which results in errors:

```
ValueError: Attempt to use ZIP archive that was already closed
```

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants