Skip to content

Replace the remote HDF5 reference translation layer with native HDF5 indexing and direct fsspec range reads - #712

Merged
FrancescAlted merged 39 commits into
mainfrom
remote-hdf5
Sep 17, 2026
Merged

FrancescAlted merged 39 commits into
mainfrom
remote-hdf5

Conversation

@FrancescAlted

Copy link
Copy Markdown
Member

Remote files are scanned once with h5py to record dataset metadata and allocated chunk ranges. Uncompressed, deflate, shuffle, and Blosc2 pipelines are decoded directly. Other HDF5 filters use a retained h5py fallback, including filters registered by hdf5plugin.

The native index is shared across RemoteStore leaves and persisted in array carriers and disk caches, avoiding repeated metadata scans on warm opens. The public hdf5_index= argument accepts an existing native index; legacy reference maps are rejected with migration guidance.

This also removes the former translation dependency from the HDF5, development, and test dependency groups. Remote HDF5 now requires only h5py and fsspec, plus the appropriate fsspec protocol backend.

Copilot AI 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.

🟡 Changes recommended

Unresolved critical and moderate findings remain in HDF5 validation, serialization, decoding, and resource lifecycle handling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Replaces kerchunk-based remote HDF5 translation with native h5py indexing, fsspec range reads, and direct filter decoding.

Changes:

  • Adds versioned index persistence and sharing across arrays and stores.
  • Supports direct deflate, shuffle, and Blosc2 decoding with h5py fallback.
  • Removes kerchunk dependencies and updates tests and documentation.
File summaries
File Description
tests/test_remote_store.py Updates index-sharing tests.
tests/test_remote_array.py Renames reference terminology.
tests/test_hdf5_source.py Adds native reader and persistence coverage.
tests/test_fsspec.py Updates HDF5 index option tests.
tests/test_b2z_source.py Adjusts optional dependency isolation.
tests/b2view/test_hierarchy.py Tests native HDF5 discovery.
src/blosc2/schunk.py Propagates the public hdf5_index API.
src/blosc2/remote_store.py Uses shared native indexes and manages source lifecycle.
src/blosc2/remote_array.py Persists and shares native indexes.
src/blosc2/hdf5_source.py Implements scanning, decoding, validation, and fallback handling.
pyproject.toml Removes kerchunk dependencies.
plans/remote-hdf5.md Documents the native-reader architecture.
doc/reference/remotestore.rst Updates RemoteStore documentation.
doc/reference/remotearray.rst Documents native HDF5 indexing.
doc/reference/hdf5ndsource.rst Documents direct and fallback pipelines.
doc/guides/remote_arrays.md Updates remote HDF5 guidance.
doc/guides/b2view.rst Updates HDF5 browser requirements.
doc/getting_started/installation.rst Updates installation guidance.
Review details

Suppressed comments (8)

src/blosc2/hdf5_source.py:326

  • available_datasets() now unconditionally imports fsspec for every .json path. Because fsspec is optional and local native index paths are supported, available_datasets("/local/index.json") fails in an hdf5-only installation even though the same path can be loaded by HDF5NDSource; use the built-in open() for paths without a URL scheme and reserve fsspec for remote indexes.
    if url_str.endswith(".json"):
        import fsspec

        with fsspec.open(url_str, "r", **(storage_options or {})) as file:
            return sorted(validate_hdf5_index(json.load(file))["datasets"])

src/blosc2/hdf5_source.py:423

  • After _open_local() this object owns a live h5py file, but an exception from _init_geometry() or the subsequent identity setup leaves __init__ without explicitly closing _file_finalizer. The previous scoped setup closed local files on failed initialization; retaining the handle until garbage collection can leak descriptors and Windows file locks on the existing invalid-dtype/geometry error paths.
        if self._local:
            self._open_local()
            self._metadata = None
            shape, physical_chunks, dtype = self.array.shape, self.array.chunks, self.array.dtype

src/blosc2/hdf5_source.py:418

  • This condition routes any local source with an explicit hdf5_index through the remote branch, which calls check_hdf5_dependencies() and requires fsspec. That breaks the documented local indexed-reader case for users who installed only blosc2[hdf5]; keep indexed local paths on the h5py/local path or make the dependency contract explicit.
        self._local = (not remote or os.path.isabs(urlpath)) and hdf5_index is None and _filesystem is None

src/blosc2/hdf5_source.py:461

  • The URL split searches the entire URL, including its query string, when extracting the dataset. A direct call such as HDF5NDSource('https://host/file.h5/group?token=x', ...) therefore treats group?token=x as the dataset (or reports a mismatch) instead of preserving the query on the file URL, so signed/query-bearing URLs fail unless callers pre-normalize them.
        for ext in (".h5/", ".hdf5/"):
            index = lower.find(ext)
            if index != -1:
                end, embedded = index + len(ext) - 1, urlpath[index + len(ext) :].strip("/")
                if dataset is not None and dataset != embedded:

src/blosc2/hdf5_source.py:242

  • The legacy-map detector only recognizes keys ending in /.zarray. A flat legacy reference map for a root dataset uses the key .zarray, so it reaches the generic Invalid HDF5 index format error and misses the promised migration guidance. Include the root .zarray/.zgroup forms in this check.
    if index.get("format") != HDF5_INDEX_FORMAT:
        if "refs" in index or any(str(key).endswith("/.zarray") for key in index):
            raise ValueError(
                "Legacy HDF5 reference maps are unsupported; omit hdf5_index and rescan the source"
            )

src/blosc2/remote_array.py:712

  • This new standalone-close path does not update any closed state, while _check_open() only detects closed store-derived handles. After RemoteArray.close(), a local HDF5 source fails unpredictably on the next read, whereas a remote fallback can reopen its HDF5 file; the same API is therefore neither consistently closed nor rejected. Mark standalone handles closed and enforce that in _check_open(), or preserve the documented no-op semantics.
        elif isinstance(getattr(self, "src", None), blosc2.HDF5NDSource):
            self.src.close()

src/blosc2/remote_array.py:712

  • Standalone RemoteArray.close() calls HDF5NDSource.close() without taking _operation_lock, while __getitem__ and fetch serialize source use with that lock. For a local HDF5 source this can close the h5py file while an in-flight read is using self.array; serialize this close with the same operation lock.
        elif isinstance(getattr(self, "src", None), blosc2.HDF5NDSource):
            self.src.close()

tests/test_fsspec.py:773

  • zarr is still imported with pytest.importorskip() immediately after the removed kerchunk guard in both HDF5 HTTP tests. Since this reader no longer uses Zarr, those tests silently skip when validating the new h5py/fsspec-only dependency set; remove the remaining Zarr guard so the range-reader coverage runs with the declared dependencies.
    h5py = pytest.importorskip("h5py")
    pytest.importorskip("zarr")
  • Files reviewed: 18/18 changed files
  • Comments generated: 7
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/blosc2/hdf5_source.py
Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/hdf5_source.py
Comment thread src/blosc2/remote_store.py
Comment thread src/blosc2/hdf5_source.py
@FrancescAlted

Copy link
Copy Markdown
Member Author

Copilot's review also listed 8 suppressed findings. All are addressed in this branch:

  • available_datasets() reads a local index JSON with the built-in open() instead of importing fsspec — 688a6f1
  • A failed HDF5NDSource.__init__ now closes the h5py file explicitly on the geometry/identity error paths — 3086f64
  • Local sources with an explicit hdf5_index keep using h5py, so blosc2[hdf5]-only installs work — 72eb847
  • .h5/ dataset parsing preserves query strings instead of treating ?token=... as part of the dataset — 2ec6bb4
  • Legacy flat .zarray/.zgroup reference maps are detected and reported with the migration message — 6f460ce
  • RemoteArray.close() marks the handle closed and rejects later operations — dcb4fa9
  • Standalone RemoteArray.close() takes the operation lock before closing its HDF5 source — dcb4fa9
  • The leftover zarr guard was removed from the two HDF5 HTTP tests — 0fb504f

Copilot AI 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.

🟡 Changes recommended

Critical HDF5 decoding-validation findings and additional lifecycle, serialization, and option-validation issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (8)

src/blosc2/hdf5_source.py:206

  • When _filesystem is omitted, _filesystem_and_path() creates an fsspec filesystem here, but this function only closes the HDF5/file handles. Public scan_hdf5_index() and the remote available_datasets() path can therefore retain owned HTTP/S3 sessions after every scan; track ownership and close the owned session in a finally, while leaving supplied filesystems open.
    fs, path = _filesystem_and_path(urlpath, storage_options, _filesystem)
    groups, datasets = {"": {"attrs": {}}}, {}
    with fs.open(path, "rb", block_size=1, cache_type="none") as raw:

src/blosc2/hdf5_source.py:664

  • The new explicit close() path does not coordinate with direct reads: _direct_values() uses self._filesystem.cat_file() without the lifecycle lock, while close() can close the session and set _filesystem to None. A standalone source closed concurrently with a fetch can therefore fail mid-read or dereference the cleared filesystem; add close-versus-read coordination without serializing independent direct fetches unnecessarily.
    def close(self):
        with self._fallback_lock:
            fallback_finalizer = getattr(self, "_fallback_finalizer", None)
            if fallback_finalizer is not None:
                fallback_finalizer()
            self._fallback_h5 = self._fallback_file = None
            finalizer = getattr(self, "_file_finalizer", None)
            if finalizer is not None:
                finalizer()
            filesystem = getattr(self, "_filesystem", None)
            if filesystem is not None and self._external_filesystem is None:
                # fsspec's HTTP and S3 clients keep an async session alive after
                # the file objects it feeds are gone.
                close = getattr(filesystem, "close_session", None)
                session = getattr(filesystem, "_s3creator", None) or getattr(filesystem, "_session", None)
                if close is not None and session is not None:
                    close(filesystem.loop, session)
                self._filesystem = None

src/blosc2/hdf5_source.py:157

  • This helper gets the default fsspec instance-cached filesystem, but HDF5NDSource.close() now closes that filesystem's session whenever _external_filesystem is None. Two standalone sources for the same HTTP/S3 URL can therefore share one cached client and closing either invalidates the other. Create a private filesystem (skip_instance_cache=True) for source-owned transports, or avoid closing shared instances.
    return fsspec.core.url_to_fs(urlpath, **(storage_options or {}))

src/blosc2/msgpack_utils.py:86

  • The new ndarray msgpack extension has the same nested-object reconstruction problem: object arrays containing ndarray/list elements are inferred as a multidimensional or ragged array before reshape(shape), so exporting such an HDF5 attribute can fail or change its structure. Rebuild object arrays by assigning each decoded value into an np.empty(shape, dtype=object) result.
    if "values" in payload:
        return np.array(payload["values"], dtype=object).reshape(shape)

src/blosc2/msgpack_utils.py:77

  • The nested payload is encoded with raw packb and no default hook, so object arrays containing supported values such as np.int64, complex numbers, sets, or nested ndarrays fail with TypeError even though the outer msgpack serializer handles them. Use the project extension hook for this nested pack as well.
    return ExtType(_BLOSC2_NDARRAY_EXT_CODE, packb(payload, use_bin_type=True))

src/blosc2/remote_array.py:557

  • RemoteArray.__init__ now exposes the public hdf5_index argument, but the class docstring's parameter list does not document it. Direct RemoteArray(...) users need the accepted native-index format and the legacy-map migration behavior; the open() docstring alone does not cover this constructor.
        hdf5_index=None,

src/blosc2/schunk.py:2235

  • Because this is popped before _validate_non_lazy_fsspec_options, open(..., lazy=False, cache_dir=..., hdf5_index=...) still raises the generic cache_policy and max_cache_bytes require lazy=True error instead of the option-specific hdf5_index error expected by the new regression test. With no cache directory, the popped index can also be ignored on the eager path. Reject a non-lazy hdf5_index explicitly before the generic cache-policy validation.
    hdf5_index = kwargs.pop("hdf5_index", None)

tests/test_hdf5_source.py:362

  • These new Blosc2-filter tests are skipped in the repository's default test job: .github/workflows/build.yml installs only --group test, while pyproject.toml's test group includes h5py but not hdf5plugin. As a result, the new direct Blosc2 path (and the fallback plugin path below) can remain unexecuted in CI; add the plugin to the required test environment or run a required plugin-filter job.
    plugin = pytest.importorskip("hdf5plugin")
  • Files reviewed: 19/19 changed files
  • Comments generated: 5
  • Review effort level: Lite

Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/hdf5_source.py
Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/hdf5_source.py Outdated
@FrancescAlted

Copy link
Copy Markdown
Member Author

Second review round: the 5 threaded findings are fixed and resolved. The 8 suppressed findings are also addressed:

  • scan_hdf5_index() now creates private filesystems (skip_instance_cache=True) and closes an owned scan session in a finally, leaving supplied filesystems untouched — 8e43464
  • HDF5NDSource.close() drains in-flight direct reads (counter + condition) before closing the session, and rejects reads after close — 7ea7d76
  • HDF5NDSource owns a private fsspec filesystem, so closing one source cannot invalidate another source for the same URL — 8e43464
  • Object-dtype msgpack arrays are rebuilt into np.empty(shape, dtype=object) and filled by flat index — 8fe4359
  • Nested ndarray payloads are packed/unpacked with the project extension hooks, so np.int64, complex, sets and nested arrays survive — 8fe4359
  • RemoteArray now documents its public hdf5_index parameter, including the legacy-map migration behavior — 1899913
  • Non-lazy hdf5_index requests are rejected explicitly before the generic cache-option validation, including the no-cache-dir eager path — 9b51abd
  • hdf5plugin is now part of the test dependency group, so the Blosc2-filter tests run in the default CI job — 3c7511d

Copilot AI 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.

🟡 Changes recommended

Critical routing and filesystem-ownership issues, plus additional validation and lifecycle defects, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

src/blosc2/hdf5_source.py:302

  • This validation treats missing dataset fields as defaults (shape=() and chunks=None) and does not require fill_value, attrs, or the other fields emitted by the scanner. Consequently an incomplete explicit index can pass validation; for example, a direct sparse entry without fill_value fails later with a KeyError, while a fallback entry without shape can be opened with the wrong geometry. Reject incomplete dataset entries before applying the type checks.

src/blosc2/hdf5_source.py:510

  • The source-owned filesystem is created before the remote index load and dataset validation, but those operations are outside the try/self.close() block starting below. If scanning, loading a malformed index, or rejecting a missing dataset raises, this standalone source leaks its HTTP/S3 session because the constructor never closes the filesystem it created. Put the remote setup under the same cleanup boundary (or close it in an exception path).
            self._filesystem, self._path = _filesystem_and_path(urlpath, storage_options, _filesystem)
            self._hdf5_index = self._load_or_scan_index(hdf5_index)
            self._validate_dataset_presence(dataset)

src/blosc2/hdf5_source.py:663

  • The fill-value path runs before the closed-state check below. After closing a sparse direct HDF5 source, requests for an unallocated chunk therefore return its fill value successfully, while requests for allocated chunks raise RuntimeError; the new close contract is inconsistent for valid sparse datasets. Check that the source is open before looking up record and construct/return the fill result while holding the lifecycle lock so close cannot race that path.
        record = self._chunk_records.get(offsets)
        valid_shape = tuple(item.stop - item.start for item in selection)
        if record is None:
            return np.full(valid_shape, _from_json_value(self._metadata["fill_value"]), dtype=self.dtype)

src/blosc2/hdf5_source.py:509

  • Remote sources do not register a finalizer for their owned filesystem: only local files get _file_finalizer, and _fallback_finalizer closes HDF5/raw handles but not the fsspec session. A standalone HDF5NDSource or RemoteArray abandoned without an explicit close() can therefore leave an HTTP/S3 session alive, especially before the first fallback read. Add an ownership-aware filesystem finalizer and detach it in close().
            self._filesystem, self._path = _filesystem_and_path(urlpath, storage_options, _filesystem)
            self._hdf5_index = self._load_or_scan_index(hdf5_index)

src/blosc2/hdf5_source.py:756

  • This catches every OSError from the retained h5py/fsspec reader and rewrites it as a filter/plugin installation failure. HTTP transport errors, permission failures, and corrupt files can therefore be reported as “install hdf5plugin,” hiding the actionable original cause; preserve non-filter errors and add plugin guidance only for filter-registration failures.
            with self._fallback_lock:
                try:
                    values = self._open_fallback()[selection]
                except OSError as exc:
                    filters = [item["id"] for item in self._metadata["filters"]]
                    raise OSError(
                        f"Cannot decode HDF5 dataset {self.dataset!r} with filters {filters}; "
                        "install hdf5plugin if the file uses an optional HDF5 filter"
                    ) from exc

src/blosc2/hdf5_source.py:744

  • After HDF5NDSource.close() closes a local file, _local remains true and this branch still dereferences self.array without a closed-state check. Unlike the remote direct/fallback branches, a closed local source therefore raises h5py's backend-specific invalid-identifier error (and can race a local read) instead of consistently rejecting the operation as closed. Track a closed state and check it before local reads.
        if self._local:
            values = self.array[selection]

src/blosc2/remote_array.py:729

  • This new standalone branch marks every RemoteArray as closed, including B2Z, Zarr, and plain fsspec arrays, even though it only needs to close an owned HDF5 source. That regresses the existing documented no-op lifetime for standalone non-HDF5 handles: simply exiting their context now makes later operations raise RuntimeError. Restrict the closed-state transition here to standalone HDF5 sources and update the public lifecycle documentation to match.
            with self._operation_lock:
                # Serialize against in-flight reads before closing the source.
                self._closed = True
                if isinstance(getattr(self, "src", None), blosc2.HDF5NDSource):
                    self.src.close()
  • Files reviewed: 19/19 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread src/blosc2/hdf5_source.py Outdated
Comment thread src/blosc2/schunk.py
Comment thread src/blosc2/hdf5_source.py Outdated
@FrancescAlted

Copy link
Copy Markdown
Member Author

Third review round: the 3 threaded findings are fixed and resolved. The 7 suppressed findings are also addressed:

  • Dataset entries must now include shape, dtype, chunks, fill_value, attrs, filters, direct and allocated; a non-list attrs/chunks is rejected before type checks — 1eaa5e2
  • Remote filesystem creation, index loading and dataset-presence validation now run inside the constructor's cleanup boundary, and an owned filesystem gets a weakref.finalize that is detached in close() — cd47460
  • Sparse fill chunks check the closed state under the lifecycle lock, and local reads are counted like direct reads so close() drains them before closing the file — 0a427b0
  • Only filter-related OSErrors get the hdf5plugin hint; transport, permission and corruption errors keep their original cause — f3c3ddb
  • Standalone RemoteArray.close() marks the handle closed only for owned HDF5 sources, preserving the no-op close semantics of B2Z/Zarr/fsspec handles — 6b0ef98

Copilot AI 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.

🔵 Needs a closer look

Unresolved issues remain in local-file dependency handling, index format resolution, sidecar validation, and HDF5 lifecycle documentation.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/blosc2/hdf5_source.py:161

  • scan_hdf5_index is documented as handling local or remote containers, but it calls check_hdf5_dependencies() and then _filesystem_and_path() unconditionally imports fsspec. Since the hdf5 extra intentionally does not install fsspec and local HDF5 access is advertised as h5py-only, calling the public scanner on a local path fails before it can use h5py. Select local-file handling first, checking only h5py there and retaining fsspec for remote URLs.
    src/blosc2/remote_array.py:561
  • hdf5_index is now a public RemoteArray parameter, but this constructor still resolves source_format only from the URL and dataset. Consequently RemoteArray("memory://container", dataset="data", hdf5_index=index) is rejected as a non-HDF5 dataset, while supplying the index with a detected Zarr/Blosc2 URL silently ignores it. Make the constructor's resolver force HDF5 for a non-conflicting index and reject conflicting formats, matching _open_fsspec_url.

src/blosc2/remote_array.py:714

  • This changes standalone HDF5 RemoteArray context exits to close the source, but the public open() notes in src/blosc2/schunk.py still say that exiting a standalone RemoteArray context is a no-op. Users following that documentation can reuse a handle after with and now receive RuntimeError; update the API note to distinguish HDF5 handles from other standalone RemoteArrays.
    def close(self):
        """Release this handle and any HDF5 file resources it owns.

        Closing is idempotent. Standalone HDF5 handles reject further
        operations; other standalone handles keep their no-op close behavior.

src/blosc2/remote_array.py:627

  • The shared sidecar is described as disposable, but only decompression/JSON syntax errors are suppressed here. A valid JSON object with an invalid format, version, URL binding, or dataset entry is assigned to hdf5_index; HDF5NDSource then raises during validation instead of rescanning, so a damaged sidecar can prevent future opens. Validate the candidate before assigning it (and discard it on validation failure).
            if hdf5_index is None and shared_index_path is not None:
                # Disposable metadata: an absent or damaged snapshot needs a fresh scan.
                with contextlib.suppress(OSError, ValueError, RuntimeError):
                    hdf5_index = json.loads(blosc2.decompress(shared_index_path.read_bytes()))
  • Files reviewed: 19/19 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@FrancescAlted

Copy link
Copy Markdown
Member Author

Fourth review round (no inline threads were generated). The 4 suppressed findings are addressed:

  • scan_hdf5_index() now selects local-file handling first and scans with the built-in open(), so a public local scan works with blosc2[hdf5] only; fsspec stays required for remote URLs — 2b7dc6d
  • RemoteArray.__init__ now folds a supplied hdf5_index into source_format (forcing HDF5 and rejecting detected Zarr/Blosc2 or Caterva2 conflicts), matching _open_fsspec_url — d181515
  • The blosc2.open() notes now state that standalone HDF5 RemoteArray handles close their source on context exit and reject further reads, while other standalone handles stay no-ops — e6cf229
  • The shared .hdf5-index.b2 sidecar is validated with validate_hdf5_index before being assigned, so a valid-JSON-but-invalid snapshot triggers a rescan instead of blocking later opens — 202395a

@FrancescAlted
FrancescAlted merged commit 35cbede into main Sep 17, 2026
42 checks passed
@FrancescAlted
FrancescAlted deleted the remote-hdf5 branch September 17, 2026 13:19
@FrancescAlted
FrancescAlted restored the remote-hdf5 branch September 17, 2026 13:30
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.

2 participants