Skip to content

Improve error messages when export of internal DataPipes fails #718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions +tests/+unit/dataPipeTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,77 @@ function testOverrideBoundPipeProperties(testCase)
testCase.verifyEqual(ME.identifier, 'NWB:BoundPipe:CannotSetPipeProperty')
end
end

function testBoundPipeExportToNewFileError(testCase)
% Test error message when exporting bound DataPipe to new file

% Create original file with DataPipe
originalFile = 'test_bound_original.nwb';
newFile = 'test_bound_new.nwb';

nwb = tests.factory.NWBFile();

fData = randi(250, 10, 100);
fData_compressed = types.untyped.DataPipe('data', fData);

fdataNWB = types.core.TimeSeries( ...
'data', fData_compressed, ...
'data_unit', 'mV', ...
'starting_time', 0.0, ...
'starting_time_rate', 30.0);

nwb.acquisition.set('test_data', fdataNWB);
nwbExport(nwb, originalFile);

% Read the file (creates a bound DataPipe)
file = nwbRead(originalFile, 'ignorecache');

% Try to export to new file - this should fail, because the
% data pipe in the imported file object is a "bound" pipe (the data
% is not in memory), and the bound pipe's write method can not
% "pipe" the data into a new file.
testCase.verifyError(@() nwbExport(file, newFile), ...
'NWB:BoundPipe:CannotExportToNewFile');
end

function testUnboundPipeExportToExistingFileError(testCase)
% Test error message when exporting "unbound" DataPipe to existing file

existingFile = 'test_unbound_existing.nwb';

% Create first file with DataPipe
nwb1 = tests.factory.NWBFile();

fData1 = randi(250, 10, 100);
fData1_compressed = types.untyped.DataPipe('data', fData1);

fdataNWB1 = types.core.TimeSeries( ...
'data', fData1_compressed, ...
'data_unit', 'mV', ...
'starting_time', 0.0, ...
'starting_time_rate', 30.0);

nwb1.acquisition.set('test_data', fdataNWB1);
nwbExport(nwb1, existingFile);

% Create second NWB object with same structure
nwb2 = tests.factory.NWBFile();

fData2 = randi(250, 10, 100);
fData2_compressed = types.untyped.DataPipe('data', fData2);
fdataNWB2 = types.core.TimeSeries( ...
'data', fData2_compressed, ...
'data_unit', 'mV', ...
'starting_time', 0.0, ...
'starting_time_rate', 30.0);
nwb2.acquisition.set('test_data', fdataNWB2);

% Try to export to existing file - this will fail, because a
% dataset already exists in the acquisition/test_data/test
% location.
testCase.verifyError(@() nwbExport(nwb2, existingFile), ...
'NWB:BlueprintPipe:DatasetAlreadyExists');
end
end

methods (Test, TestTags={'UsesDynamicallyLoadedFilters'})
Expand Down
26 changes: 25 additions & 1 deletion +types/+untyped/+datapipe/BlueprintPipe.m
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,31 @@ function removePipeProperty(obj, type)
end
dcpl = obj.makeDcpl();
dapl = 'H5P_DEFAULT';
did = H5D.create(fid, fullpath, tid, sid, lcpl, dcpl, dapl);

try
did = H5D.create(fid, fullpath, tid, sid, lcpl, dcpl, dapl);
catch ME
if contains(ME.message, "name already exists")
H5P.close(dcpl);
H5S.close(sid);
if ~ischar(tid)
H5T.close(tid);
end
error('NWB:BlueprintPipe:DatasetAlreadyExists', ...
['Cannot export an unbound DataPipe to an existing file that already ' ...
'contains the dataset at path "%s". ' ...
'To fix this: either export to a new file, or use export mode "overwrite" ' ...
'to replace the existing file, or read the existing file and modify it in place.'], ...
fullpath);
else
H5P.close(dcpl);
H5S.close(sid);
if ~ischar(tid)
H5T.close(tid);
end
rethrow(ME);
end
end

H5D.close(did);
H5P.close(dcpl);
Expand Down
20 changes: 18 additions & 2 deletions +types/+untyped/+datapipe/BoundPipe.m
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,24 @@ function removePipeProperty(~, ~)
'Bound pipes cannot remove pipe properties.');
end

function obj = write(obj, ~, ~)
return;
function obj = write(obj, fid, fullpath)
% Check if the link exists
exists = H5L.exists(fid, fullpath, 'H5P_DEFAULT');
assert(exists, ...
'NWB:BoundPipe:CannotExportToNewFile', ...
['Cannot export the dataset (%s) to a new NWB file.\n', ...
'The dataset you are trying to export is stored in a ', ...
'different file ("%s"), and the data is not physically ', ...
'included in the current NWB file object. When exporting to ', ...
'a new file, NWB needs a complete copy of the data — but ', ...
'in this case, it only has a reference to the data in the ', ...
'original file.\n', ...
'To fix this, you have two options:\n', ...
'1. Export to the same file ("%s"), which already ', ...
'contains the data.\n', ...
'2. Load the data into memory and create a new dataset ' ...
'that includes the full data, so it can be saved in the ', ...
'new file.'], fullpath, obj.filename, obj.filename);
end

function data = load(obj, varargin)
Expand Down
Loading