Skip to content

Copy DataStub datasets through the backend writer - #868

Open
ehennestad wants to merge 2 commits into
zarr-support/6-writer-reference-probefrom
zarr-support/7-writer-dataset-copy
Open

Copy DataStub datasets through the backend writer#868
ehennestad wants to merge 2 commits into
zarr-support/6-writer-reference-probefrom
zarr-support/7-writer-dataset-copy

Conversation

@ehennestad

@ehennestad ehennestad commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Background — This PR is part of a series that makes the write path storage-agnostic (stacked on #867), so that a Zarr v3 writer can be added later.

Problem — When a file is read and then exported to a new file, datasets whose data still lives in the source file (types.untyped.DataStub) are copied over during export. This copy was done with raw HDF5 calls inside @DataStub/export.m: a check whether source and destination are the same file, a fast object copy, and a workaround that rewrites compound datasets containing references because an HDF5 library bug prevents copying them directly. All of this only works with the HDF5 backend, and a writer for another storage format cannot reuse any of it.

Solution — Move the copy into the writer. io.backend.base.Writer gains copyDatasetFromFile(sourceFilename, sourcePath, destinationPath), and the HDF5 code moves unchanged into io.backend.hdf5.HDF5Writer. DataStub.export now only calls this method. Re-exporting a read file behaves the same as before.

What changed

  • New method copyDatasetFromFile(sourceFilename, sourcePath, destinationPath) on io.backend.base.Writer, implemented in io.backend.hdf5.HDF5Writer.
  • types.untyped.DataStub.export contains no HDF5 calls and no writer.FileId access; it calls the writer method.
  • Copy behavior is unchanged: nothing is copied when source and destination are the same file, or when the destination path already exists. Compound datasets containing references are still rewritten element by element.
  • Fixed in passing: the old code did not close the source file id when it returned early on the same-file check. The moved code closes it.
  • Added the new method to the expected-methods list in the base writer test.
Implementation notes

Ordinary datasets are copied with H5O.copy. Compound datasets containing references cannot be copied this way (HDFGroup/hdf5#3429), so they are read with io.parseCompound and written again through writeValue, as before. The base-class help states that an implementation may replace the raw copy with a rewrite when its storage layer cannot copy a dataset directly.

How to test

Read a file and export it to a new file; the data, which is still on disk in the source file, is carried over:

src = nwbRead('ref_demo.nwb', 'ignorecache');
nwbExport(src, 'ref_demo_copy.nwb');
copyRead = nwbRead('ref_demo_copy.nwb', 'ignorecache');
disp(copyRead.acquisition.get('ts').data.load().')
     1     2     3     4     5     6     7     8     9    10

(ref_demo.nwb is the file created in the test snippet of #867.)

Checklist

  • Have you ensured the PR description clearly describes the problem and solutions?
  • Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
  • If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (zarr-support/6-writer-reference-probe@7033673). Learn more about missing BASE report.

Files with missing lines Patch % Lines
+io/+backend/+hdf5/HDF5Writer.m 93.75% 2 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                            @@
##             zarr-support/6-writer-reference-probe     #868   +/-   ##
========================================================================
  Coverage                                         ?   95.26%           
========================================================================
  Files                                            ?      234           
  Lines                                            ?     8315           
  Branches                                         ?        0           
========================================================================
  Hits                                             ?     7921           
  Misses                                           ?      394           
  Partials                                         ?        0           

☔ 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.

@ehennestad
ehennestad force-pushed the zarr-support/7-writer-dataset-copy branch from 07e906b to 906309c Compare August 26, 2026 17:43
ehennestad and others added 2 commits August 31, 2026 12:42
DataStub.export reached through the abstract writer to a raw HDF5 file
id and drove the copy itself with H5F/H5D/H5T/H5O calls: skip when the
source and destination are the same file, rewrite element-wise when the
dataset is compound with reference members (HDF5 library bug, see
HDFGroup/hdf5#3429), otherwise fast-copy via
H5O.copy when the destination does not exist yet.

Give the base Writer a copyDatasetFromFile method stating those
semantics in backend-neutral terms and move the HDF5 mechanics into
HDF5Writer unchanged, including the same-file check via canonical
H5F.get_name names. DataStub.export shrinks to a single delegation and
no longer touches writer.FileId or any H5* function.

The moved code also closes the source file id on the same-file early
return, which the original leaked.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ehennestad
ehennestad force-pushed the zarr-support/7-writer-dataset-copy branch from 906309c to 1de59d8 Compare August 31, 2026 10:42
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.

1 participant