Skip to content

Conversation

bwalker1
Copy link
Contributor

@bwalker1 bwalker1 commented Jun 10, 2025

Fixes #197

Changed storagesize implementation for DirectoryStore so it doesn't break when no chunks are initialized by replacing sum(.) with reduce(., init=0). Also changed DictStore implementation to match even though it was technically not broken before.

@coveralls
Copy link

coveralls commented Jun 10, 2025

Pull Request Test Coverage Report for Build 17361585816

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 2 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+3.3%) to 88.611%

Files with Coverage Reduction New Missed Lines %
src/ZArray.jl 1 94.86%
src/metadata.jl 5 84.71%
Totals Coverage Status
Change from base Build 15493524096: 3.3%
Covered Lines: 957
Relevant Lines: 1080

💛 - Coveralls

Copy link
Collaborator

@meggart meggart left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix. Ideally, adding a test for an empty array where staoragesize errors on current master would be great to avoid breaking things again in the future, but can as well be merged IMO.

asinghvi17 added a commit that referenced this pull request Aug 31, 2025
This commit adds tests that demonstrate the issue reported in #197
where calling zinfo() on an empty array (with no initialized chunks)
fails with an ArgumentError on DirectoryStore.

The tests will fail on master but pass after PR #198 is merged.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Member

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I added a test in #209.

@asinghvi17 asinghvi17 mentioned this pull request Aug 31, 2025
Copy link
Member

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

Sum takes init too

asinghvi17

This comment was marked as duplicate.

@asinghvi17 asinghvi17 requested a review from meggart September 2, 2025 21:18
@meggart meggart merged commit e826e37 into JuliaIO:master Sep 9, 2025
14 checks passed
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.

zinfo(z) fails when no chunks of z are initialized
4 participants