Claude/internal simplifications

[d-v-b]

I had claude do a review of our codebase and look for low-hanging fruit style improvements

Summary

Internal-only simplifications: dedupe repeated creation / containment logic and remove single-caller pass-through wrappers. Zero public-API changes — all touched code is private (underscore-prefixed or module-internal) with no external callers (verified via grep across src/ and tests/). Behavior is identical; verified with targeted tests and ad-hoc snippets where stack/precedence semantics mattered.

Addresses S1–S6, S8, S10 of https://www.github.com/d-v-b/zarr-python/issues/181.

Changes (each is a separate commit)

1. Dedupe overwrite-check logic (src/zarr/core/array.py) — extracted the repeated "delete existing node if overwriting and the store supports deletes, else enforce no existing node" dance into a private _prepare_overwrite helper, used by _create_v3, _create_v2, and _create_array_metadata. The three sites used two equivalent spellings; both collapse to the helper. (+19 / −18)

2. Remove single-caller get-selection wrappers (src/zarr/core/array.py) — _get_orthogonal_selection, _get_mask_selection, _get_coordinate_selection each built an indexer and delegated to _get_selection, with exactly one caller apiece (the matching AsyncArray.get_*_selection). Inlined the indexer construction into those methods and deleted the wrappers. (+19 / −182). No analogous module-level _set_*_selection wrappers exist, so nothing to remove there.

3. Unify dtype dispatch (src/zarr/core/array.py) — ArrayV2Metadata.dtype and ArrayV3Metadata.dtype (the latter an alias for data_type) both already return the zarr dtype object, so the inline metadata.dtype if zarr_format == 2 else metadata.data_type dispatch in AsyncArray._zdtype, _get_selection, and _set_selection collapses to metadata.dtype. No new property needed. (+7 / −14)

4. cdata_shape duplication (src/zarr/core/array.py) — Array.cdata_shape now delegates to Array._chunk_grid_shape instead of duplicating its body. Signatures/docstrings unchanged. (+1 / −1)

5. "not yet implemented" warning boilerplate (src/zarr/api/asynchronous.py) — extracted a _warn_unimplemented_kwargs(dict) helper used by open_group and create. Uses stacklevel=3 to compensate for the extra frame; verified the emitted warning has identical message, category, source location, and emission order as before. (+32 / −22)

6. contains_group V3 dedup (src/zarr/storage/_common.py) — rewrote the V3 branch as (await _contains_node_v3(store_path)) == "group", behaviorally identical across all cases (missing doc, array, group, malformed/non-object JSON, missing node_type, invalid format). (+1 / −12)

   • contains_array intentionally left unchanged: its V3 branch falls through to raise ValueError("Invalid zarr_format...") when a group node is present, which _contains_node_v3 does not reproduce (it would yield False). Swapping it would change observable behavior, so it was skipped to honor the "behavior identical" constraint.

7. Inline no-op default_filters_v3 (src/zarr/core/array.py) — the function unconditionally returned (). Inlined the empty tuple at both call sites (note: two callers, not one) with a short comment and deleted the function. (+4 / −11)

8. Fold fill_value into _like_args (src/zarr/api/asynchronous.py) — _like_args now records the source array's fill_value, so empty_like/full_like/open_like drop their isinstance + setdefault blocks. ones_like/zeros_like pop the inherited fill_value (they supply their own), preserving exact prior precedence semantics — including the pre-existing duplicate-keyword TypeError when an explicit fill_value is passed to ones_like/zeros_like. Updated the _like_args unit test to include the new key. (+11 / −8)

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.54%. Comparing base (e29ddd2) to head (9ca2870).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4105      +/-   ##
==========================================
+ Coverage   93.50%   93.54%   +0.03%     
==========================================
  Files          90       90              
  Lines       11981    11937      -44     
==========================================
- Hits        11203    11166      -37     
+ Misses        778      771       -7     

Files with missing lines	Coverage Δ
src/zarr/api/asynchronous.py	96.29% <100.00%> (+2.24%)	⬆️
src/zarr/core/array.py	97.83% <100.00%> (-0.04%)	⬇️
src/zarr/storage/_common.py	89.10% <100.00%> (-0.42%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[maxrjones]

TBH I find the PR title confusing, as I'm not sure what I should take from it starting with Claude. Is this a new commit style?

[maxrjones]

The Claude-generated summary makes it hard to gauge how much to scrutinize. For an internal refactor this size you could self-merge per our policy. What are you hoping a second reviewer adds? where would you most want me to look?

[d-v-b]

The Claude-generated summary makes it hard to gauge how much to scrutinize. For an internal refactor this size you could self-merge per our policy. What are you hoping a second reviewer adds? where would you most want me to look?

sorry for the confusion, I was just looking for another set of eyes on these changes, since some of them are a bit closer to public API than other internal cleanups. I think a quick eyeball check on them would be fine! And i'm happy self-reviewing if you don't have time.

It's a collection of logically independent changes, so in principle they could each be in a separate PR, but I opted for batching them. If that strategy doesn't work well, I can avoid this in the future.