Skip to content

Add DASDAE storage/codec API with h5py-native compression - #817

Open
d-chambers wants to merge 6 commits into
devfrom
dasdae-storage-dev
Open

Add DASDAE storage/codec API with h5py-native compression#817
d-chambers wants to merge 6 commits into
devfrom
dasdae-storage-dev

Conversation

@d-chambers

@d-chambers d-chambers commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a storage/codec API for DASDAE writes, ported to dev's h5py-based IO stack. This supersedes #734, which was written against master's PyTables implementation.

Codecs describe how array payloads are compressed; storage objects describe how a writer applies those codecs plus layout options such as per-dimension chunking. Keeping the two separate means chunk layout stays a storage concern and codecs remain reusable by other HDF5-backed formats.

User API

Simple preset compression:

patch.io.write(path, "dasdae", storage="compressed")

Dict form with a codec and chunk layout (no imports needed):

patch.io.write(
    path,
    "dasdae",
    storage={"codec": {"name": "gzip", "level": 5}, "chunks": {"time": 2000}},
)

Typed form:

from dascore.io.dasdae import DASDAEStorage
from dascore.io.hdf5 import Gzip

storage = DASDAEStorage(codec=Gzip(level=5), chunks={"time": 2000})
patch.io.write(path, "dasdae", storage=storage)

Capability discovery:

dc.io.get_storage("DASDAE")   # -> DASDAEStorage
dc.io.get_codecs("DASDAE")    # -> (Gzip,)

Implementation

  • BaseCodec and BaseStorage pydantic models in dascore.io.core, plus get_storage()/get_codecs() discovery. FiberIO.storage_cls is derived from the storage annotation on write() so the storage type has a single source of truth.
  • A plugin-extensible codec registry (dascore.codec entry-point group) in dascore.io.codec; only get_codecs is exported on the dascore.io namespace to avoid a get_codec/get_codecs naming trap.
  • An h5py-native Gzip codec in dascore.io.hdf5. Blosc/zstd is not included for now: h5py has no built-in blosc filter, so the compressed preset uses gzip level 5. A future hdf5plugin-backed codec can restore it through the registry without API changes.
  • DASDAEStorage with codec/chunks options and fail-fast validation: unknown codec names, codec instances without a registered discriminator, non-positive chunk sizes, and typoed chunk dimension names all raise before any data is written. Chunk-dim validation uses scan metadata so lazy spools are not materialized twice.
  • Codec and chunk layout apply to data arrays and coordinate arrays (chunks match coordinates by dim name); scalar and zero-length arrays fall back to contiguous storage.
  • Patch data is now decoded through the same attribute-aware path as coordinates on read, so string and datetime data arrays round-trip exactly (previously they came back as raw bytes/int64), including through selective reads.

Compatibility

Files written without a storage argument are byte-for-byte equivalent to before. Compressed files use native HDF5 filters, so any HDF5 reader (h5py, PyTables, external tools) can read them without DASCore.

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes.
  • documented the new feature with docstrings and/or appropriate doc page.
  • included tests. See testing guidelines.
  • added the "ready_for_review" tag once the PR is ready to be reviewed.

Summary by CodeRabbit

  • New Features

    • Added configurable DASDAE storage options for compression and chunking.
    • Added built-in gzip compression with adjustable compression level and shuffle settings.
    • Added codec discovery and lookup, including support for registered extensions.
    • Added validation for storage settings, codecs, and chunk dimensions.
    • Improved handling of scalar, empty, Unicode, date/time, and filtered data.
  • Documentation

    • Added guidance and examples for configuring DASDAE compression.

Port the storage/codec design from #734 (originally PyTables-based, written
against master) onto dev's h5py-only DASDAE implementation:

- BaseCodec/BaseStorage models plus get_storage()/get_codecs() discovery
  in dascore.io.core, with storage_cls derived from write() annotations.
- A plugin-extensible codec registry (dascore.codec entry-point group).
- An h5py-native Gzip codec; blosc:zstd is dropped for now since h5py has
  no built-in blosc filter (it can return later via an hdf5plugin-backed
  codec or plugin).
- DASDAEStorage with codec/chunks options, presets, and fail-fast
  validation; DASDAEV1.write(storage=...) applies compression and chunk
  layout to data and coordinate arrays.
- Decode patch data through the same attr-aware path as coordinates so
  string/datetime data arrays round-trip instead of silently coming back
  as raw bytes/ints.
Chunk-dim validation iterated the spool, fully loading every patch of a
lazy spool once for validation and again for the write; use scan metadata
instead. Also cover the selective-read decode branch with string and
datetime data arrays, which no test exercised.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@d-chambers, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 72b16b47-0eaa-43bb-878f-74ad0d253061

📥 Commits

Reviewing files that changed from the base of the PR and between a4cb0f0 and 7922eb5.

📒 Files selected for processing (8)
  • dascore/io/codec.py
  • dascore/io/core.py
  • dascore/io/dasdae/core.py
  • dascore/io/netcdf/core.py
  • pyproject.toml
  • tests/test_io/test_dasdae/test_dasdae.py
  • tests/test_io/test_io_core.py
  • tests/test_io/test_prodml/test_prodml_write.py
📝 Walkthrough

Walkthrough

Changes

Storage and codec flow

Layer / File(s) Summary
Codec contracts and registry
dascore/io/codec.py, dascore/io/hdf5.py, pyproject.toml, dascore/io/core.py, dascore/io/__init__.py
Adds BaseCodec, HDF5 codecs, built-in and entry-point discovery, codec lookup, and package-level exports.
FiberIO storage wiring
dascore/io/core.py, tests/test_io/test_io_core.py
Adds storage model discovery, storage capability reporting, input coercion, format lookup, and related tests.
DASDAE storage and array I/O
dascore/io/dasdae/*, dascore/io/hdf5.py, tests/test_io/test_dasdae/*, tests/test_io/test_hdf5.py, docs/tutorial/file_io.qmd
Adds DASDAE codec and chunk configuration, applies dataset options during writes, restores encoded array types during reads, and documents and tests the new forms.

Possibly related PRs

  • DASDAE/dascore#733: Both changes configure HDF5 compression in DASDAEV1.write and DASDAE utility functions.

Suggested labels: IO, patch

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding the DASDAE storage and codec API with h5py-native compression.
Description check ✅ Passed The description explains the API, implementation, compatibility, documentation, tests, and related issue context; only optional checklist items remain unchecked.
Docstring Coverage ✅ Passed Docstring coverage is 95.88% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dasdae-storage-dev

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added IO Work for reading/writing different formats patch related to Patch class labels Aug 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
docs/tutorial/file_io.qmd (1)

59-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: evaluate the dict storage example.

Both the dict form and the typed form use #| eval: false, so the render never checks them. The example patch has a time dimension with 2000 samples, so this cell can run as written. Removing eval: false here makes the docs fail fast if the storage contract changes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/tutorial/file_io.qmd` around lines 59 - 66, Remove the #| eval: false
directive from the dict-form patch.io.write example so the cell executes during
documentation rendering. Keep the existing write_path, codec, compression level,
and time chunk configuration unchanged.
dascore/io/dasdae/utils.py (1)

191-193: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: reuse _decode_array_values in _read_array_sample.

_read_array_sample repeats the three decode branches that _decode_array_values now centralizes. It can delegate and index the result, which keeps one decode contract for both readers.

Proposed consolidation
 def _read_array_sample(table_array, index):
     """Read one array sample and restore datetime-like dtypes when needed."""
-    out = table_array[index]
-    attrs = table_array.attrs
-    if attrs.get("is_datetime64"):
-        out = np.asarray([out]).view("datetime64[ns]")[0]
-    if attrs.get("is_timedelta64"):
-        out = np.asarray([out]).view("timedelta64[ns]")[0]
-    if attrs.get("is_string"):
-        original_dtype = unbyte(attrs.get("original_string_dtype", ""))
-        out = convert_bytes_to_strings(np.asarray([out]), original_dtype)[0]
-    return out
+    out = np.asarray([table_array[index]])
+    return _decode_array_values(out, table_array.attrs)[0]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dascore/io/dasdae/utils.py` around lines 191 - 193, Update _read_array_sample
to delegate decoding to _decode_array_values and then apply its sample/index
selection to the decoded result, removing the duplicated decode branches while
preserving the existing sampled-reader behavior and _read_array’s contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dascore/io/codec.py`:
- Around line 57-59: After the loader is executed and assigned to codec_cls on
line 58, validate that codec_cls is a subclass of BaseCodec before proceeding to
call _codec_name and register it in the registry. Skip or reject loader results
that do not meet this requirement to prevent invalid entries from being added to
the codec registry.

In `@dascore/io/dasdae/core.py`:
- Around line 88-92: Update the dims extraction in the storage validation flow
to reuse the existing normalization logic for catalog `dims` values before
calling `storage._validate_chunk_dims(all_dims)`. Parse each comma-separated
string into normalized dimension names rather than stringifying tuple-like
values, while preserving the `storage.chunks` guard and empty-scan behavior.

In `@dascore/io/dasdae/storage.py`:
- Around line 121-145: Update _resolve_chunkshape to return None whenever any
value in shape is zero, before constructing the chunk tuple. Preserve the
existing None behavior for missing chunks or mismatched dims, and leave
_dataset_options unchanged so empty arrays avoid zero-sized HDF5 chunk
dimensions.

In `@tests/test_io/test_io_core.py`:
- Around line 806-808: Update test_builtins_registered and the corresponding
assertions around the additional built-in codecs to isolate registry behavior
from installed plugins by stubbing get_entry_point_loaders() to return no
plugins. Keep the exact built-in identity assertions once plugin discovery is
disabled.

---

Nitpick comments:
In `@dascore/io/dasdae/utils.py`:
- Around line 191-193: Update _read_array_sample to delegate decoding to
_decode_array_values and then apply its sample/index selection to the decoded
result, removing the duplicated decode branches while preserving the existing
sampled-reader behavior and _read_array’s contract.

In `@docs/tutorial/file_io.qmd`:
- Around line 59-66: Remove the #| eval: false directive from the dict-form
patch.io.write example so the cell executes during documentation rendering. Keep
the existing write_path, codec, compression level, and time chunk configuration
unchanged.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1fb22d1e-4207-461e-95ca-b8fb14be4195

📥 Commits

Reviewing files that changed from the base of the PR and between 0afc948 and a4cb0f0.

📒 Files selected for processing (13)
  • dascore/io/__init__.py
  • dascore/io/codec.py
  • dascore/io/core.py
  • dascore/io/dasdae/__init__.py
  • dascore/io/dasdae/core.py
  • dascore/io/dasdae/storage.py
  • dascore/io/dasdae/utils.py
  • dascore/io/hdf5.py
  • docs/tutorial/file_io.qmd
  • pyproject.toml
  • tests/test_io/test_dasdae/test_dasdae.py
  • tests/test_io/test_hdf5.py
  • tests/test_io/test_io_core.py

Comment thread dascore/io/codec.py Outdated
Comment on lines +57 to +59
for loader in get_entry_point_loaders(_CODEC_ENTRY_POINT_GROUP).values():
codec_cls = loader()
registry[_codec_name(codec_cls)] = codec_cls

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject loader results that are not codec classes.

Line 58 accepts any loader result. A loader that returns Gzip() or an unrelated class can produce a malformed registry or fail later outside the plugin boundary. Validate that the result is a BaseCodec subclass before calling _codec_name.

Proposed fix
     for loader in get_entry_point_loaders(_CODEC_ENTRY_POINT_GROUP).values():
         codec_cls = loader()
+        if not isinstance(codec_cls, type) or not issubclass(codec_cls, BaseCodec):
+            msg = "Codec entry points must return a BaseCodec subclass."
+            raise InvalidFiberIOError(msg)
         registry[_codec_name(codec_cls)] = codec_cls
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for loader in get_entry_point_loaders(_CODEC_ENTRY_POINT_GROUP).values():
codec_cls = loader()
registry[_codec_name(codec_cls)] = codec_cls
for loader in get_entry_point_loaders(_CODEC_ENTRY_POINT_GROUP).values():
codec_cls = loader()
if not isinstance(codec_cls, type) or not issubclass(codec_cls, BaseCodec):
msg = "Codec entry points must return a BaseCodec subclass."
raise InvalidFiberIOError(msg)
registry[_codec_name(codec_cls)] = codec_cls
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dascore/io/codec.py` around lines 57 - 59, After the loader is executed and
assigned to codec_cls on line 58, validate that codec_cls is a subclass of
BaseCodec before proceeding to call _codec_name and register it in the registry.
Skip or reject loader results that do not meet this requirement to prevent
invalid entries from being added to the codec registry.

Comment thread dascore/io/dasdae/core.py Outdated
Comment on lines +88 to +92
if storage.chunks:
scan_df = dc.scan_to_df(patches)
dims_rows = scan_df["dims"] if "dims" in scan_df.columns else ()
all_dims = {dim for row in dims_rows for dim in str(row).split(",") if dim}
storage._validate_chunk_dims(all_dims)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how the dims column is produced on the Spool.get_contents path.
rg -nP -C4 '"dims"' dascore/core/spool.py dascore/core/summary.py dascore/io/index/catalog.py

Repository: DASDAE/dascore

Length of output: 4756


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file outline =="
ast-grep outline dascore/io/dasdae/core.py --view expanded || true

echo "== dasdae core lines 60-115 =="
sed -n '60,115p' dascore/io/dasdae/core.py

echo "== scan_to_df definitions/usages =="
rg -n "def scan_to_df|scan_to_df\\(" dascore -g '*.py'

echo "== Spool get_contents outline/definition =="
ast-grep outline dascore/core/spool.py --match get_contents --view expanded || true
rg -n -C4 "def get_contents|get_contents\\(" dascore/core/spool.py dascore -g '*.py' | head -220

Repository: DASDAE/dascore

Length of output: 9479


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== dascore/core/summary.py relevant lines =="
sed -n '1,115p' dascore/core/summary.py
sed -n '255,280p' dascore/core/summary.py

echo "== dascore/utils/patch.py scan_to_patches_dataframe lines =="
sed -n '350,385p' dascore/utils/patch.py

echo "== dascore/io/core.py scan_to_df lines =="
sed -n '1220,1242p' dascore/io/core.py
rg -n -C4 "def scan\\(|class.*Spool|class DataSpool|to_df|_catalog\\.to_df" dascore/core dascore/io/index/catalog.py dascourse/core/spool.py | head -260

echo "== Read-only behavioral probe of dims parsing for current implementation =="
python3 - <<'PY'
# Simulate the current dims parsing on representative row contents.
def parse(row):
    return {dim for dim in str(row).split(",") if dim}
rows = [
    ("time",),
    ["time", "distance"],
    {"dtype": "float64", "dims": ("time", "distance")},
    "time,distance",
]
for row in rows:
    print({type(row).__name__: parse(row)})
PY

Repository: DASDAE/dascore

Length of output: 16057


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== catalog to_df implementation =="
sed -n '889,925p' dascore/io/index/catalog.py

echo "== Search dims column from catalog flat rel =="
rg -n -C3 '"dims"' dascore/io/index/catalog.py dascore/io/index/indexer.py dascore/utils/patch.py dascore/utils/downloader.py dascore/io/dasdae/core.py dascore/core/spool.py | head -220

echo "== behavioral probe with current parsing =="
python3 - <<'PY'
def current_parse(dims_rows):
    all_dims = {dim for row in dims_rows for dim in str(row).split(",") if dim}
    return sorted(all_dims)

rows = [
    ("time",),
    ["time", "distance"],
    "time,distance",
    {},
]

print("tuple =", current_parse(rows[:1]))
print("list =", current_parse(rows[1:2]))
print("str =", current_parse(rows[2:3]))
print("no dims =", current_parse(rows[3:]))
PY

Repository: DASDAE/dascore

Length of output: 4255


Parse the dims column as a comma-separated string.

dc.scan_to_df() returns the spool index as-is, and catalog rows expose dims as the string value. Splitting with str(row).split(",") produces garbage values like "('distance'", so configured chunk dims are rejected as unknown. Use the same normalization logic already used elsewhere before calling _validate_chunk_dims(); keep the empty-scan guard so an empty spool with chunks does not fail.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dascore/io/dasdae/core.py` around lines 88 - 92, Update the dims extraction
in the storage validation flow to reuse the existing normalization logic for
catalog `dims` values before calling `storage._validate_chunk_dims(all_dims)`.
Parse each comma-separated string into normalized dimension names rather than
stringifying tuple-like values, while preserving the `storage.chunks` guard and
empty-scan behavior.

Comment on lines +121 to +145
if self.chunks is None or not shape or len(dims) != len(shape):
return None
return tuple(
min(self.chunks.get(dim, size), size)
for dim, size in zip(dims, shape, strict=True)
)

def _dataset_options(self, dims, shape) -> dict:
"""
Return h5py ``create_dataset`` options for one array.

Combines the codec's compression kwargs with the chunk shape resolved
for the array's dims. An empty dict means default contiguous storage.
"""
codec = self.codec
if codec is None:
out = {}
else:
# _resolve_codec guarantees only HDF5-filter codecs get this far.
assert isinstance(codec, HDF5Codec)
out = codec._dataset_kwargs()
chunkshape = self._resolve_chunkshape(dims, shape)
if chunkshape is not None:
out["chunks"] = chunkshape
return out

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python - <<'PY'
import tempfile
import h5py

with tempfile.NamedTemporaryFile(suffix=".h5") as tmp:
    with h5py.File(tmp.name, "w") as h5:
        try:
            h5.create_dataset("empty", shape=(0,), dtype="f8", chunks=(0,))
        except Exception as exc:
            print(f"Rejected zero-sized chunk dimension: {type(exc).__name__}")
        else:
            raise AssertionError("h5py accepted an invalid zero-sized chunk dimension")
PY

Repository: DASDAE/dascore

Length of output: 269


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== candidate files =="
fd -a 'storage\.py$' . | sed 's#^\./##' | grep dascore/io/dasdae/storage.py || true

echo "== source excerpt =="
sed -n '1,220p' dascore/io/dasdae/storage.py

echo "== related references =="
rg -n "_resolve_chunkshape|_dataset_options|chunks|dset|create_dataset|HDF5Codec|storage" dascore/io/dasdae -S

Repository: DASDAE/dascore

Length of output: 9532


🌐 Web query:

h5py create_dataset chunks must be positive zero values documentation

💡 Result:

In h5py, the chunks parameter for create_dataset must consist of positive integer values because it defines the shape of the data blocks stored on disk [1][2][3]. HDF5 requires that chunk dimensions be strictly greater than zero; a value of zero in any dimension of the chunk shape will result in a ValueError with the message, "All chunk dimensions must be positive" [4][5][6]. This requirement is enforced by the underlying HDF5 library's property list settings [4][7][5]. When specifying chunks manually, you must provide a tuple of positive integers that matches the rank (number of dimensions) of the dataset [1][2][7]. Key points regarding this requirement: - Chunked storage relies on dividing a dataset into regular, fixed-size pieces [2][8]. A dimension of zero is logically incompatible with the definition of a fixed-size chunk [4][6]. - If you encounter this error, ensure that none of the values in the tuple passed to the chunks argument are 0 or negative [4][5]. - If you are working with datasets that involve dynamic or unlimited dimensions, you should use None for the unlimited axes in the shape or maxshape parameters, but the chunk size for any active dimension must still be a positive integer [9][4]. - If you are unsure what chunk shape to use, you can pass chunks=True to allow h5py to automatically determine an appropriate chunk shape for you [1][8].

Citations:


Do not emit zero-sized chunk dimensions.

_resolve_chunkshape(dims, shape=(..., 0, ...)) currently returns a chunk tuple such as (..., 0, ...), while HDF5 dataset chunks must be strictly positive integers. Return None when any array dimension is empty so the existing empty-array write path can use contiguous storage or automatic chunking.

Proposed fix
-        if self.chunks is None or not shape or len(dims) != len(shape):
+        if (
+            self.chunks is None
+            or not shape
+            or any(size == 0 for size in shape)
+            or len(dims) != len(shape)
+        ):
             return None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if self.chunks is None or not shape or len(dims) != len(shape):
return None
return tuple(
min(self.chunks.get(dim, size), size)
for dim, size in zip(dims, shape, strict=True)
)
def _dataset_options(self, dims, shape) -> dict:
"""
Return h5py ``create_dataset`` options for one array.
Combines the codec's compression kwargs with the chunk shape resolved
for the array's dims. An empty dict means default contiguous storage.
"""
codec = self.codec
if codec is None:
out = {}
else:
# _resolve_codec guarantees only HDF5-filter codecs get this far.
assert isinstance(codec, HDF5Codec)
out = codec._dataset_kwargs()
chunkshape = self._resolve_chunkshape(dims, shape)
if chunkshape is not None:
out["chunks"] = chunkshape
return out
if (
self.chunks is None
or not shape
or any(size == 0 for size in shape)
or len(dims) != len(shape)
):
return None
return tuple(
min(self.chunks.get(dim, size), size)
for dim, size in zip(dims, shape, strict=True)
)
def _dataset_options(self, dims, shape) -> dict:
"""
Return h5py ``create_dataset`` options for one array.
Combines the codec's compression kwargs with the chunk shape resolved
for the array's dims. An empty dict means default contiguous storage.
"""
codec = self.codec
if codec is None:
out = {}
else:
# _resolve_codec guarantees only HDF5-filter codecs get this far.
assert isinstance(codec, HDF5Codec)
out = codec._dataset_kwargs()
chunkshape = self._resolve_chunkshape(dims, shape)
if chunkshape is not None:
out["chunks"] = chunkshape
return out
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dascore/io/dasdae/storage.py` around lines 121 - 145, Update
_resolve_chunkshape to return None whenever any value in shape is zero, before
constructing the chunk tuple. Preserve the existing None behavior for missing
chunks or mismatched dims, and leave _dataset_options unchanged so empty arrays
avoid zero-sized HDF5 chunk dimensions.

Comment thread tests/test_io/test_io_core.py
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (acb25ca) to head (7922eb5).

Additional details and impacted files
@@            Coverage Diff             @@
##               dev      #817    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          164       167     +3     
  Lines        17916     18133   +217     
==========================================
+ Hits         17916     18133   +217     
Flag Coverage Δ
network 48.35% <37.65%> (-0.14%) ⬇️
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Document the storage kwarg on dc.write/patch.io.write, the entry point
  the tutorial teaches.
- Warn and skip broken dascore.codec entry points instead of letting one
  stale plugin poison the registry (mirrors the FiberIO plugin loader).
- Validate chunk dims before writing file meta so a typo no longer leaves
  a DASDAE-stamped stub; let empty spools write with chunk config instead
  of rejecting every chunk dim.
- Give targeted errors when a codec name is passed as a preset or a codec
  instance is passed as storage.
- Fix stale blosc:zstd example in the BaseStorage presets comment.
# Conflicts:
#	tests/test_io/test_io_core.py
A typo in a write option name (e.g. stroage='compressed') previously
vanished into the writer's **kwargs and produced a file that ignored
what the caller asked for. dc.write/patch.io.write now validate kwargs
against the named parameters of the target format's write() and raise
ParameterError listing the supported options. Read kwargs are untouched
(they are open-ended by design for attr filtering).

NetCDF's docstring-only compression/compression_opts/chunks options are
promoted to named parameters so they participate in the contract.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IO Work for reading/writing different formats patch related to Patch class

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant