Skip to content

Commit 541db1d

Browse files
ehennestadCopilot
andauthored
Refactor / simplify DataStub.export method (#767)
* Simplify DataStub/export * Add boolean type to compound dataset for improved testing * Update +types/+untyped/@DataStub/export.m Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
1 parent 7cdde97 commit 541db1d

File tree

3 files changed

+49
-52
lines changed

3 files changed

+49
-52
lines changed

+tests/+unit/dataStubTest.m

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,20 @@ function testObjectCopy(testCase)
8383
rcRef = types.cs.CompoundRefData('data', table(...
8484
rand(2, 1),...
8585
rand(2, 1),...
86+
[true; false],...
8687
[types.untyped.ObjectView(rcPath); types.untyped.ObjectView(rcPath)],...
8788
[types.untyped.RegionView(rcDataPath, 1:2, 99:100); types.untyped.RegionView(rcDataPath, 5:6, 88:89)],...
88-
'VariableNames', {'a', 'b', 'objref', 'regref'}));
89+
'VariableNames', {'a', 'b', 'c', 'objref', 'regref'}));
8990

9091
nwb.acquisition.set('rc', rc);
9192
nwb.analysis.set('rcRef', rcRef);
9293
nwbExport(nwb, 'original.nwb');
93-
nwbNew = nwbRead('original.nwb', 'ignorecache');
94-
tests.util.verifyContainerEqual(testCase, nwbNew, nwb);
95-
nwbExport(nwbNew, 'new.nwb');
94+
nwbOriginalIn = nwbRead('original.nwb', 'ignorecache');
95+
tests.util.verifyContainerEqual(testCase, nwbOriginalIn, nwb);
96+
97+
nwbExport(nwbOriginalIn, 'copy.nwb');
98+
nwbCopyIn = nwbRead('copy.nwb', 'ignorecache');
99+
tests.util.verifyContainerEqual(testCase, nwbCopyIn, nwb);
96100
end
97101

98102
function testLoadWithEmptyIndices(testCase)
@@ -151,9 +155,10 @@ function testResolveCompoundDataType(testCase)
151155
compoundRef = types.cs.CompoundRefData('data', table(...
152156
rand(2, 1),...
153157
rand(2, 1),...
158+
[true; false],...
154159
[types.untyped.ObjectView(tsPath); types.untyped.ObjectView(tsPath)],...
155160
[types.untyped.RegionView(tsDataPath, 1:2); types.untyped.RegionView(tsDataPath, 2:3)],...
156-
'VariableNames', {'a', 'b', 'objref', 'regref'}));
161+
'VariableNames', {'a', 'b', 'c', 'objref', 'regref'}));
157162

158163
nwb.analysis.set('compoundRef', compoundRef);
159164

+tests/test-schema/compoundSchema/cs.compoundtypes.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ groups:
1010
- name: b
1111
dtype: float64
1212
doc: 'B'
13+
- name: c
14+
dtype: bool
15+
doc: 'C'
1316
- name: objref
1417
doc: 'ObjectView in Compound datatype'
1518
dtype:

+types/+untyped/@DataStub/export.m

Lines changed: 36 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
function refs = export(obj, fid, fullpath, refs)
2-
%Check for compound data type refs
2+
3+
% If exporting to the same file this DataStub originates from, skip export.
34
src_fid = H5F.open(obj.filename);
4-
% if filenames are the same, then do nothing
55
src_filename = H5F.get_name(src_fid);
66
dest_filename = H5F.get_name(fid);
77
if strcmp(src_filename, dest_filename)
@@ -10,53 +10,28 @@
1010

1111
src_did = H5D.open(src_fid, obj.path);
1212
src_tid = H5D.get_type(src_did);
13-
src_sid = H5D.get_space(src_did);
14-
ref_i = false;
15-
char_i = false;
16-
member_name = {};
17-
ref_tid = {};
13+
14+
% Check for compound data type refs
1815
if H5T.get_class(src_tid) == H5ML.get_constant_value('H5T_COMPOUND')
19-
ncol = H5T.get_nmembers(src_tid);
20-
ref_i = false(ncol, 1);
21-
member_name = cell(ncol, 1);
22-
char_i = false(ncol, 1);
23-
ref_tid = cell(ncol, 1);
24-
refTypeConst = H5ML.get_constant_value('H5T_REFERENCE');
25-
strTypeConst = H5ML.get_constant_value('H5T_STRING');
26-
for i = 1:ncol
27-
member_name{i} = H5T.get_member_name(src_tid, i-1);
28-
subclass = H5T.get_member_class(src_tid, i-1);
29-
subtid = H5T.get_member_type(src_tid, i-1);
30-
char_i(i) = subclass == strTypeConst && ...
31-
~H5T.is_variable_str(subtid);
32-
if subclass == refTypeConst
33-
ref_i(i) = true;
34-
ref_tid{i} = subtid;
35-
end
36-
end
16+
isCompoundDatasetWithReference = isCompoundWithReference(src_tid);
17+
else
18+
isCompoundDatasetWithReference = false;
3719
end
38-
39-
%manually load the data struct
40-
if any(ref_i)
41-
%This requires loading the entire table.
42-
%Due to this HDF5 library's inability to delete/update
43-
%dataset data, this is unfortunately required.
44-
ref_tid = ref_tid(~cellfun('isempty', ref_tid));
20+
21+
% If dataset is compound and contains reference types, data needs to be
22+
% manually read and written to the new file. This is due to a bug in
23+
% the hdf5 library (see e.g. https://github.com/HDFGroup/hdf5/issues/3429)
24+
if isCompoundDatasetWithReference
25+
% This requires loading the entire table.
26+
% Due to this HDF5 library's inability to delete/update
27+
% dataset data, this is unfortunately required.
4528
data = H5D.read(src_did);
46-
47-
refNames = member_name(ref_i);
48-
for i=1:length(refNames)
49-
data.(refNames{i}) = io.parseReference(src_did, ref_tid{i}, ...
50-
data.(refNames{i}));
51-
end
52-
53-
strNames = member_name(char_i);
54-
for i=1:length(strNames)
55-
s = data.(strNames{i}) .';
56-
data.(strNames{i}) = mat2cell(s, ones(size(s,1),1));
57-
end
58-
29+
30+
% Use io.parseCompound to consistently handle references, character arrays, and logical types,
31+
% ensuring all data types are properly postprocessed in line with the rest of the codebase.
32+
data = io.parseCompound(src_did, data);
5933
io.writeCompound(fid, fullpath, data);
34+
6035
elseif ~H5L.exists(fid, fullpath, 'H5P_DEFAULT')
6136
% copy data over and return destination.
6237
ocpl = H5P.create('H5P_OBJECT_COPY');
@@ -66,7 +41,21 @@
6641
H5P.close(lcpl);
6742
end
6843
H5T.close(src_tid);
69-
H5S.close(src_sid);
7044
H5D.close(src_did);
7145
H5F.close(src_fid);
72-
end
46+
end
47+
48+
function hasReference = isCompoundWithReference(src_tid)
49+
hasReference = false;
50+
51+
ncol = H5T.get_nmembers(src_tid);
52+
refTypeConst = H5ML.get_constant_value('H5T_REFERENCE');
53+
54+
for i = 1:ncol
55+
subclass = H5T.get_member_class(src_tid, i-1);
56+
if subclass == refTypeConst
57+
hasReference = true;
58+
return
59+
end
60+
end
61+
end

0 commit comments

Comments
 (0)