Skip to content

Conversation

@oelhammouchi
Copy link

Fix a bug where ZipStore complains that it has no attribute _lock when some methods are called. Any method using the lock should make sure the store is opened because that's when the _lock attribute gets set.

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Nov 21, 2025
@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.94%. Comparing base (7d8105f) to head (64940a1).

Files with missing lines Patch % Lines
src/zarr/storage/_zip.py 50.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3593      +/-   ##
==========================================
- Coverage   61.95%   61.94%   -0.02%     
==========================================
  Files          86       86              
  Lines       10170    10182      +12     
==========================================
+ Hits         6301     6307       +6     
- Misses       3869     3875       +6     
Files with missing lines Coverage Δ
src/zarr/storage/_zip.py 70.39% <50.00%> (-1.47%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@d-v-b
Copy link
Contributor

d-v-b commented Nov 21, 2025

how did the bug you are fixing here get past the store tests? Maybe we need to make those tests stronger?

@oelhammouchi
Copy link
Author

You're right, I didn't want to appear presumptuous but the code could do with some refactoring as well, having to repeat all those _sync_open calls in so many functions naturally increases the probability of forgetting some. Not sure if this should be done in this PR though.

@d-v-b
Copy link
Contributor

d-v-b commented Nov 22, 2025

if you can find a clean way to reduce the repetition (e.g., a decorator) we'd appreciate it!

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

Labels

needs release notes Automatically applied to PRs which haven't added release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants