Skip to content

Conversation

@melonora
Copy link

@melonora melonora commented Dec 4, 2025

In the codebase _iter_shard_regions was already falling back to chunks in case there are no shards. Another method, iter_shard_coords did not have this fallback option. This PR adds a fallback uption for that method in case there is no shard grid.

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@d-v-b @joshmoore

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Dec 4, 2025
@d-v-b
Copy link
Contributor

d-v-b commented Dec 4, 2025

we need separate methods for iterating over chunks and shards because they are not the same

# /// script
# requires-python = ">=3.11"
# dependencies = [
#   "zarr@git+https://github.com/zarr-developers/zarr-python.git@main",
# ]
# ///

import zarr

array = zarr.create_array({} ,shape=(10,), dtype='i4', chunks=(5,), shards=(10,), fill_value=0)

print(tuple(array._iter_shard_regions()))
# ((slice(0, 10, 1),),)
print(tuple(array._iter_chunk_regions()))
# ((slice(0, 5, 1),), (slice(5, 10, 1),))

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.14%. Comparing base (ee0e69a) to head (eded573).

Files with missing lines Patch % Lines
src/zarr/core/array.py 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3614      +/-   ##
==========================================
- Coverage   59.14%   59.14%   -0.01%     
==========================================
  Files          86       86              
  Lines       10172    10174       +2     
==========================================
+ Hits         6016     6017       +1     
- Misses       4156     4157       +1     
Files with missing lines Coverage Δ
src/zarr/core/array.py 68.44% <57.14%> (-0.04%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@melonora
Copy link
Author

melonora commented Dec 4, 2025

I see that, but do not see where this is used internally that it matters. Otherwise I would just provide the same fallback option for _iter_shard_coords and leave the original _iter_chunk intact.

@d-v-b
Copy link
Contributor

d-v-b commented Dec 4, 2025

we do not use these methods internally yet, but that's not a good reason for removing them.

also see #3573

@melonora
Copy link
Author

melonora commented Dec 4, 2025

we do not use these methods internally yet, but that's not a good reason for removing them.

also see #3573

Then I will revert them, but provide just the same fallback option for `_iter_shard_coords'

@melonora melonora changed the title Remove _iter_chunks methods/functions Provide chunk_grid_space fallback in _iter_shard_coords Dec 4, 2025
@d-v-b
Copy link
Contributor

d-v-b commented Dec 4, 2025

I'm confused by these changes -- is there a bug that this fixes? Because we should already fall back to the chunk grid for unsharded v2 arrays, via the _shard_grid_shape property

@melonora melonora closed this Dec 4, 2025
@melonora
Copy link
Author

melonora commented Dec 4, 2025

Thanks @d-v-b for your patience and pointing it out. Was mainly based on spotting it based on the other method.

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