Skip to content

Commit 8126412

Browse files
ehennestadclaude
andcommitted
refactor: make embedded-spec read/write backend-agnostic
io.spec.writeEmbeddedSpecifications, io.spec.validateEmbeddedSpecifications, and io.spec.readEmbeddedSpecifications were hardcoded to raw HDF5 file IDs (H5G.open, H5L.delete, h5info, H5F.open, H5D.open/read), even though export/read already go through the io.backend.base.Writer/Reader abstraction everywhere else. This blocked any non-HDF5 storage backend (e.g. a Zarr v3 writer) from ever completing nwbExport/nwbRead, since NwbFile.embedSpecifications passed a raw H5-typed writer.FileId into these functions regardless of the actual backend. Adds three methods to the abstract io.backend.base.Writer interface, implemented for HDF5Writer by delegating to the existing HDF5 utility functions it already wraps: - getEmbeddedSpecLocation() (mirrors the existing Reader-side method) - listChildGroupNames(groupPath) - deleteNode(nodePath) writeEmbeddedSpecifications/validateEmbeddedSpecifications now take the writer object itself and only call through this interface. readEmbeddedSpecifications now takes the reader object and is reimplemented using the already-abstract reader.readNodeInfo/ readDatasetValue (no new Reader methods needed). NwbFile.embedSpecifications and nwbRead's generateEmbeddedSpec are updated accordingly. Verified against the full existing HDF5 test suite (517/521 passing; the 4 failures are pre-existing HDF5 dynamic-filter-availability issues on this machine, unrelated to this change) plus manual round-trip checks: fresh export, edit-mode re-export (exercises the namespace-version overwrite path), missing-namespace warning, and unused-namespace deletion. Note: this does not itself add a Zarr v3 writer/reader -- it only removes the H5-specific blocker so a future non-HDF5 backend can implement getEmbeddedSpecLocation/listChildGroupNames/deleteNode on the Writer side (the Reader side already only needs readNodeInfo/readDatasetValue, which every existing Reader implementation already provides) and get full nwbExport/nwbRead support for free. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 2ee6a6b commit 8126412

8 files changed

Lines changed: 86 additions & 53 deletions

File tree

+io/+backend/+base/Writer.m

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,27 @@ function writeAttribute(obj, attributePath, value, varargin) %#ok<INUSD>
3333
io.backend.base.Writer.throwNotImplemented("writeAttribute")
3434
end
3535

36+
function specLocation = getEmbeddedSpecLocation(obj) %#ok<MANU>
37+
% getEmbeddedSpecLocation - Return the location of embedded schema
38+
% specifications, or '' if none are embedded yet. Mirrors
39+
% io.backend.base.Reader.getEmbeddedSpecLocation for the write
40+
% side (needed when editing a file that may already embed specs).
41+
specLocation = '';
42+
io.backend.base.Writer.throwNotImplemented("getEmbeddedSpecLocation")
43+
end
44+
45+
function groupNames = listChildGroupNames(obj, groupPath) %#ok<INUSD>
46+
% listChildGroupNames - Return the names of immediate child
47+
% groups (not datasets) under groupPath.
48+
groupNames = {};
49+
io.backend.base.Writer.throwNotImplemented("listChildGroupNames")
50+
end
51+
52+
function deleteNode(obj, nodePath) %#ok<INUSD>
53+
% deleteNode - Delete the group or dataset at nodePath.
54+
io.backend.base.Writer.throwNotImplemented("deleteNode")
55+
end
56+
3657
function close(obj) %#ok<MANU>
3758
% Default no-op. Concrete backends can override when they own
3859
% resources that should be released explicitly.

+io/+backend/+hdf5/HDF5Writer.m

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,18 @@ function writeValue(obj, datasetPath, value, varargin)
8282
function writeAttribute(obj, attributePath, value, varargin)
8383
io.writeAttribute(obj.H5FileId, attributePath, value, varargin{:});
8484
end
85+
86+
function specLocation = getEmbeddedSpecLocation(obj)
87+
specLocation = io.spec.internal.readEmbeddedSpecLocation(obj.H5FileId);
88+
end
89+
90+
function groupNames = listChildGroupNames(obj, groupPath)
91+
groupNames = io.internal.h5.listGroupNames(obj.H5FileId, groupPath);
92+
end
93+
94+
function deleteNode(obj, nodePath)
95+
io.internal.h5.deleteGroup(obj.H5FileId, nodePath);
96+
end
8597
end
8698

8799
methods (Access = protected)

+io/+spec/readEmbeddedSpecifications.m

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
1-
function specs = readEmbeddedSpecifications(filename, specLocation)
1+
function specs = readEmbeddedSpecifications(reader, specLocation)
22
% readEmbeddedSpecifications - Read embedded specs from an NWB file
33
%
4-
% specs = io.spec.readEmbeddedSpecifications(filename, specLocation) read
5-
% embedded specs from the specLocation in an NWB file
4+
% specs = io.spec.readEmbeddedSpecifications(reader, specLocation) reads
5+
% embedded specs from the specLocation in an NWB file, using reader
6+
% (an io.backend.base.Reader) to access the file. Backend-agnostic:
7+
% only uses the io.backend.base.Reader interface, so this works for
8+
% any registered storage backend, not just HDF5.
69
%
710
% Inputs:
8-
% filename (string) : Absolute path of an nwb file
9-
% specLocation (string) : h5 path for the location of specs inside the NWB file
11+
% reader (io.backend.base.Reader) : Reader for the NWB file
12+
% specLocation (string) : Path for the location of specs inside the NWB file
1013
%
1114
% Outputs
1215
% specs cell: A cell array of structs with one element for each embedded
@@ -17,40 +20,38 @@
1720
% - schemaMap (containers.Map): A set of schema specifications for the namespace
1821

1922
arguments
20-
filename (1,1) string {matnwb.common.mustBeNwbFile}
23+
reader (1,1) io.backend.base.Reader
2124
specLocation (1,1) string
2225
end
2326

24-
specInfo = h5info(filename, specLocation);
27+
specInfo = reader.readNodeInfo(specLocation);
2528
specs = deal( cell(size(specInfo.Groups)) );
26-
27-
fid = H5F.open(filename);
28-
fileCleanup = onCleanup(@(id) H5F.close(fid) );
2929

3030
for iGroup = 1:length(specInfo.Groups)
31-
location = specInfo.Groups(iGroup).Groups(1);
31+
namespaceGroupInfo = specInfo.Groups(iGroup);
32+
location = namespaceGroupInfo.Groups(1);
3233

33-
namespaceName = split(specInfo.Groups(iGroup).Name, '/');
34+
namespaceName = split(namespaceGroupInfo.Name, '/');
3435
namespaceName = namespaceName{end};
3536

36-
filenames = {location.Datasets.Name};
37-
if ~any(strcmp('namespace', filenames))
37+
datasetNames = {location.Datasets.Name};
38+
if ~any(strcmp('namespace', datasetNames))
3839
warning('NWB:Read:GenerateSpec:CacheInvalid',...
3940
'Couldn''t find a `namespace` in namespace `%s`. Skipping cache generation.',...
4041
namespaceName);
4142
return;
4243
end
43-
sourceNames = {location.Datasets.Name};
44-
fileLocation = strcat(location.Name, '/', sourceNames);
44+
4545
schemaMap = containers.Map;
46-
for iFileLocation = 1:length(fileLocation)
47-
did = H5D.open(fid, fileLocation{iFileLocation});
48-
if strcmp('namespace', sourceNames{iFileLocation})
49-
namespaceText = H5D.read(did);
46+
for iDataset = 1:length(datasetNames)
47+
datasetName = datasetNames{iDataset};
48+
datasetPath = strcat(location.Name, '/', datasetName);
49+
datasetValue = reader.readDatasetValue(location.Datasets(iDataset), datasetPath);
50+
if strcmp('namespace', datasetName)
51+
namespaceText = datasetValue;
5052
else
51-
schemaMap(sourceNames{iFileLocation}) = H5D.read(did);
53+
schemaMap(datasetName) = datasetValue;
5254
end
53-
H5D.close(did);
5455
end
5556

5657
specs{iGroup}.namespaceName = namespaceName;

+io/+spec/validateEmbeddedSpecifications.m

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,33 @@
1-
function validateEmbeddedSpecifications(h5_file_id, expectedNamespaceNames)
1+
function validateEmbeddedSpecifications(writer, expectedNamespaceNames)
22
% validateEmbeddedSpecifications - Validate the embedded specifications
33
%
44
% This function does two things:
55
% 1) Displays a warning if specifications of expected namespaces
6-
% are not embedded in the file.
6+
% are not embedded in the file.
77
% E.g if cached namespaces were cleared prior to export.
8-
%
9-
% 2) Deletes specifications for unused namespaces that are embedded.
8+
%
9+
% 2) Deletes specifications for unused namespaces that are embedded.
1010
% - E.g. If neurodata type from an embedded namespace was removed and the
1111
% file was re-exported
12+
%
13+
% Backend-agnostic: only uses the io.backend.base.Writer interface, so
14+
% this works for any registered storage backend, not just HDF5.
1215

13-
% NB: Input h5_file_id must point to a file opened with write access
16+
arguments
17+
writer (1,1) io.backend.base.Writer
18+
expectedNamespaceNames
19+
end
1420

15-
specLocation = io.spec.internal.readEmbeddedSpecLocation(h5_file_id);
16-
embeddedNamespaceNames = io.internal.h5.listGroupNames(h5_file_id, specLocation);
21+
specLocation = writer.getEmbeddedSpecLocation();
22+
embeddedNamespaceNames = writer.listChildGroupNames(specLocation);
1723

1824
checkMissingNamespaces(expectedNamespaceNames, embeddedNamespaceNames)
1925

2026
unusedNamespaces = checkUnusedNamespaces(...
2127
expectedNamespaceNames, embeddedNamespaceNames);
2228

2329
if ~isempty(unusedNamespaces)
24-
deleteUnusedNamespaces(h5_file_id, unusedNamespaces, specLocation)
30+
deleteUnusedNamespaces(writer, unusedNamespaces, specLocation)
2531
end
2632
end
2733

@@ -45,10 +51,10 @@ function checkMissingNamespaces(expectedNamespaceNames, embeddedNamespaceNames)
4551
unusedNamespaces = setdiff(embeddedNamespaceNames, expectedNamespaceNames);
4652
end
4753

48-
function deleteUnusedNamespaces(fileId, unusedNamespaces, specRootLocation)
54+
function deleteUnusedNamespaces(writer, unusedNamespaces, specRootLocation)
4955
for i = 1:numel(unusedNamespaces)
5056
thisName = unusedNamespaces{i};
5157
namespaceSpecLocation = strjoin( {specRootLocation, thisName}, '/');
52-
io.internal.h5.deleteGroup(fileId, namespaceSpecLocation)
58+
writer.deleteNode(namespaceSpecLocation)
5359
end
5460
end
Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
function writeEmbeddedSpecifications(writer, jsonSpecs)
22
% writeEmbeddedSpecifications - Write schema specifications to an NWB file
3+
%
4+
% Backend-agnostic: only uses the io.backend.base.Writer interface, so
5+
% this works for any registered storage backend, not just HDF5.
36

47
arguments
58
writer (1,1) io.backend.base.Writer
69
jsonSpecs % String representation of schema specifications in json format
710
end
811

9-
specLocation = io.spec.internal.readEmbeddedSpecLocation(writer.FileId);
12+
specLocation = writer.getEmbeddedSpecLocation();
1013

1114
if isempty(specLocation)
1215
specLocation = '/specifications';
@@ -20,12 +23,9 @@ function writeEmbeddedSpecifications(writer, jsonSpecs)
2023
schemaNamespaceLocation = strjoin({specLocation, JsonDatum.name}, '/');
2124
namespaceExists = writer.writeGroup(schemaNamespaceLocation);
2225
if namespaceExists
23-
namespaceGroupId = H5G.open(writer.FileId, schemaNamespaceLocation);
24-
names = getVersionNames(namespaceGroupId);
25-
H5G.close(namespaceGroupId);
26+
names = writer.listChildGroupNames(schemaNamespaceLocation);
2627
for iNames = 1:length(names)
27-
H5L.delete(writer.FileId, [schemaNamespaceLocation '/' names{iNames}],...
28-
'H5P_DEFAULT');
28+
writer.deleteNode([schemaNamespaceLocation '/' names{iNames}]);
2929
end
3030
end
3131
schemaLocation = ...
@@ -40,13 +40,3 @@ function writeEmbeddedSpecifications(writer, jsonSpecs)
4040
end
4141
end
4242
end
43-
44-
function versionNames = getVersionNames(namespaceGroupId)
45-
[~, ~, versionNames] = H5L.iterate(namespaceGroupId,...
46-
'H5_INDEX_NAME', 'H5_ITER_NATIVE',...
47-
0, @appendName, {});
48-
function [status, versionNames] = appendName(~, name, versionNames)
49-
versionNames{end+1} = name;
50-
status = 0;
51-
end
52-
end

+tests/+unit/+io/+backend/+base/BaseWriterTest.m

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
"writeGroup", ...
99
"writeValue", ...
1010
"writeAttribute", ...
11+
"getEmbeddedSpecLocation", ...
12+
"listChildGroupNames", ...
13+
"deleteNode", ...
1114
"abort", ...
1215
"close", ...
1316
"ensure" ...

NwbFile.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ function embedSpecifications(obj, writer)
392392
jsonSpecs);
393393

394394
io.spec.validateEmbeddedSpecifications(...
395-
writer.FileId, ...
395+
writer, ...
396396
strrep(namespaceNames, '_', '-'))
397397
end
398398

nwbRead.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
end
8888
end
8989
else
90-
generateEmbeddedSpec(filename, specLocation, 'savedir', options.savedir);
90+
generateEmbeddedSpec(reader, specLocation, 'savedir', options.savedir);
9191
end
9292
else
9393
warnIfSchemaVersionsMismatch(schemaVersionOfFile, schemaVersionActive)
@@ -119,15 +119,15 @@
119119
nwb.resolveSoftLinks()
120120
end
121121

122-
function generateEmbeddedSpec(filename, specLocation, options)
122+
function generateEmbeddedSpec(reader, specLocation, options)
123123
% generateEmbeddedSpec - Generate embedded specifications / namespaces
124124
arguments
125-
filename (1,1) string {matnwb.common.compatibility.mustBeFile}
125+
reader (1,1) io.backend.base.Reader
126126
specLocation (1,1) string
127127
options.savedir (1,1) string = misc.getMatnwbDir(); % {matnwb.common.compatibility.mustBeFolder} ?
128128
end
129129

130-
specs = io.spec.readEmbeddedSpecifications(filename, specLocation);
130+
specs = io.spec.readEmbeddedSpecifications(reader, specLocation);
131131
specNames = cell(size(specs));
132132

133133
for iSpec = 1:numel(specs)

0 commit comments

Comments
 (0)