Export DataPipes through the backend writer and remove FileId - #869
Draft
ehennestad wants to merge 2 commits into
Draft
Export DataPipes through the backend writer and remove FileId#869ehennestad wants to merge 2 commits into
ehennestad wants to merge 2 commits into
Conversation
This was referenced Aug 25, 2026
ehennestad
force-pushed
the
zarr-support/8-writer-datapipe
branch
from
August 25, 2026 17:45
9692a9c to
b417932
Compare
ehennestad
force-pushed
the
zarr-support/8-writer-datapipe
branch
from
August 26, 2026 17:43
b417932 to
d97b407
Compare
Collaborator
Author
|
Internal datapipe classes are still HDF5 bound. Need decision on this. Generalise, or rename to HD5FBoundPipe and HD5FBluePrintPipe and add equivalent ZarrPipes? |
DataPipe.export handed writer.FileId to its internal pipe, the last call site reaching through the abstract writer to a raw HDF5 file id. The pipe internals (BoundPipe/BlueprintPipe) drive H5P/H5D directly for chunking, compression, dynamic filters and extendable datasets; that is legitimately HDF5-specific and stays put, but the raw id should not cross the backend interface to get there. Give the base Writer an exportDataPipe method: it writes the pipe's dataset at the destination path and returns the pipe state bound to the written dataset, which DataPipe.export stores as its new internal state. HDF5Writer implements it by passing its own H5FileId to the pipe's write method. With no consumers left, delete the FileId dependent property and the getFileId plumbing from the base Writer and HDF5Writer. HDF5WriterTest now inspects the file through HDF5Writer's own H5FileId property. Non-export DataPipe internals (the append/bind path) are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ehennestad
force-pushed
the
zarr-support/8-writer-datapipe
branch
from
August 31, 2026 10:42
d97b407 to
74c27fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Background — Last PR in the series that makes the write path storage-agnostic (stacked on #868), so that a Zarr v3 writer can be added later.
Problem —
DataPipe.exportpassed a raw HDF5 file id to its internal pipe object (obj.internal.write(writer.FileId, fullpath)). This was the last place that usedWriter.FileId. As long as the base writer exposes an HDF5 file id, export code can depend on HDF5 directly, and a writer for another storage format cannot implement the interface.Solution — Add
exportDataPipe(dataPipe, destinationPath)to the writer interface. It writes the pipe's dataset with its chunking and compression settings, and returns the pipe state bound to the written dataset. With its last user gone, theFileIdproperty is removed fromio.backend.base.Writer. After this PR, no code outside the HDF5 backend can reach an HDF5 file id. Writing Zarr v3 files is not part of this PR; that comes in a separate PR.What changed
exportDataPipe(dataPipe, destinationPath)onio.backend.base.Writer, implemented inio.backend.hdf5.HDF5Writer.types.untyped.DataPipe.exportcalls the writer method and no longer uses a file id.FileIdproperty fromio.backend.base.Writer.HDF5Writerkeeps itsH5FileId, and its unit test reads that directly.Implementation notes
exportDataPipereturns the written pipe instead of assigning it, becauseDataPipe.internalhasSetAccess = private— onlyDataPipe.exportcan assign it. The pipe classes themselves (BoundPipe,BlueprintPipe, chunking, compression, dynamic filters) stay HDF5-specific and are now only reachable from the HDF5 writer. Exporting a pipe that is bound to a dataset in a different file still raisesNWB:BoundPipe:CannotExportToNewFile.How to test
Export a chunked
DataPipeand read it back, then check that the base writer no longer has aFileIdproperty:Checklist
fix #XXwhereXXis the issue number?🤖 Generated with Claude Code