Skip to content

Commit e40e344

Browse files
ehennestadehennestad
authored
Merge branch 'main' into 650-bug-exporting-scalar-text
2 parents ee4451c + 3dfcef0 commit e40e344

26 files changed

+460
-88
lines changed

‎+file/Attribute.m

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
readonly; %determines whether value can be changed or not
88
dtype; %type of value
99
dependent; %set externally. If the attribute is actually dependent on an untyped dataset/group
10+
dependent_fullname; %set externally. This is the full name, including names of potential parent groups separated by underscore. A value will only be present if it would differ from dependent.
1011
scalar; %if the value is scalar or an array
1112
dimnames;
1213
shape;
@@ -22,6 +23,7 @@
2223
obj.readonly = false;
2324
obj.dtype = '';
2425
obj.dependent = '';
26+
obj.dependent_fullname = '';
2527
obj.scalar = true;
2628
obj.shape = {};
2729
obj.dimnames = {};

‎+file/Group.m

+12
Original file line numberDiff line numberDiff line change
@@ -245,10 +245,22 @@
245245
end
246246
PropertyMap(groupName) = [SetType, Descendant];
247247
else
248+
if isa(Descendant, 'file.Attribute')
249+
% Ad hoc convenience step: We need the parent's
250+
% expanded property name when populating the
251+
% type's class definition. Here, we construct a full
252+
% name from groupName + descendantName, then remove
253+
% the descendantName (and its underscore) and
254+
% add the result to the attribute object for
255+
% easy retrieval when needed.
256+
fullName = [groupName, '_', descendantName];
257+
Descendant.dependent_fullname = strrep(fullName, ['_', Descendant.name], '');
258+
end
248259
PropertyMap([groupName, '_', descendantName]) = Descendant;
249260
end
250261
end
251262
end
263+
252264
end
253265
end
254266
end

‎+file/fillClass.m

+1-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@
149149
classprops,...
150150
namespace, ...
151151
superClassProps);
152-
setterFcns = file.fillSetters(setdiff(nonInherited, union(readonly, hiddenAndReadonly)));
152+
setterFcns = file.fillSetters(setdiff(nonInherited, union(readonly, hiddenAndReadonly)), classprops);
153153
validatorFcns = file.fillValidators(allProperties, classprops, namespace, namespace.getFullClassName(name), inherited);
154154
exporterFcns = file.fillExport(nonInherited, class, depnm, required);
155155
methodBody = strjoin({constructorBody...

‎+file/fillExport.m

+1-1
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@
245245
% group is not required, we need to issue a warning if the parent
246246
% is set and the dependent required property is unset.
247247
isParentRequired = any(strcmp(depPropname, required));
248-
if prop.required && not(isParentRequired)
248+
if prop.required && not(prop.readonly) && not(isParentRequired)
249249
dependencyCheck{end+1} = sprintf('~isempty(obj.%s) && isempty(obj.%s)', depPropname, name);
250250
warnIfMissingRequiredDependentAttributeStr = ...
251251
sprintf('obj.warnIfRequiredDependencyMissing(''%s'', ''%s'', fullpath)', name, depPropname);

‎+file/fillProps.m

+24-1
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,21 @@
2323
proplines = cell(size(names));
2424
for i=1:length(names)
2525
pnm = names{i};
26+
propInfo = props(pnm);
2627
propStr = strjoin(strsplit(getPropStr(props(pnm)), newline), [newline '% ']);
27-
proplines{i} = [pnm '; % ' requiredStr ' ' propStr];
28+
defaultValue = [];
29+
if startsWith(class(propInfo), 'file.')
30+
if isprop(propInfo, 'value')
31+
defaultValue = propInfo.value;
32+
end
33+
end
34+
if isempty(defaultValue)
35+
proplines{i} = [pnm '; % ' requiredStr ' ' propStr];
36+
else
37+
defaultValue = formatValueAsString(defaultValue);
38+
proplines{i} = [pnm ' = %s; %% ', requiredStr ' ' propStr];
39+
proplines{i} = sprintf(proplines{i}, defaultValue);
40+
end
2841
end
2942

3043
if isempty(p.Results.PropertyAttributes)
@@ -108,4 +121,14 @@ function assertValidRefType(referenceType)
108121
word (1,:) char
109122
end
110123
word(1) = upper(word(1));
124+
end
125+
126+
function valueAsStr = formatValueAsString(value)
127+
if isnumeric(value)
128+
valueAsStr = num2str(value);
129+
elseif ischar(value)
130+
valueAsStr = sprintf("""%s""", value);
131+
else
132+
error('Not implemented. If you see this error, please report')
133+
end
111134
end

‎+file/fillSetters.m

+50-6
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,55 @@
1-
function fsstr = fillSetters(propnames)
1+
function fsstr = fillSetters(propnames, classprops)
22
fsstr = cell(size(propnames));
33
for i=1:length(propnames)
44
nm = propnames{i};
5-
fsstr{i} = strjoin({...
6-
['function set.' nm '(obj, val)']...
7-
[' obj.' nm ' = obj.validate_' nm '(val);']...
8-
'end'}, newline);
5+
prop = classprops(nm);
6+
postsetFunctionStr = resolvePostsetFunction(nm, prop);
7+
if isempty(postsetFunctionStr)
8+
fsstr{i} = strjoin({...
9+
['function set.' nm '(obj, val)']...
10+
[' obj.' nm ' = obj.validate_' nm '(val);'] ...
11+
'end'}, newline);
12+
else
13+
fsstr{i} = strjoin({...
14+
['function set.' nm '(obj, val)']...
15+
[' obj.' nm ' = obj.validate_' nm '(val);'] ...
16+
[' obj.postset_' nm '()'], ...
17+
'end', ...
18+
postsetFunctionStr}, newline);
19+
end
920
end
21+
1022
fsstr = strjoin(fsstr, newline);
11-
end
23+
end
24+
25+
function postsetFunctionStr = resolvePostsetFunction(propname, prop)
26+
27+
postsetFunctionStr = '';
28+
29+
if isa(prop, 'file.Attribute')
30+
31+
if ~isempty(prop.dependent) && ~prop.readonly
32+
33+
if ~isempty(prop.dependent_fullname)
34+
parentname = prop.dependent_fullname;
35+
else
36+
parentname = prop.dependent;
37+
end
38+
39+
conditionStr = sprintf(...
40+
'if isempty(obj.%s) && ~isempty(obj.%s)', ...
41+
parentname, propname);
42+
43+
warnIfDependencyMissingString = sprintf(...
44+
'obj.warnIfAttributeDependencyMissing(''%s'', ''%s'')', ...
45+
propname, parentname);
46+
47+
postsetFunctionStr = strjoin({...
48+
sprintf('function postset_%s(obj)', propname), ...
49+
file.addSpaces(conditionStr, 4), ...
50+
file.addSpaces(warnIfDependencyMissingString, 8), ...
51+
file.addSpaces('end', 4), ...
52+
'end'}, newline);
53+
end
54+
end
55+
end

‎+tests/+unit/WarningsTest.m

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
classdef WarningsTest < matlab.unittest.TestCase
2+
3+
methods (TestClassSetup)
4+
function setupClass(testCase)
5+
% Get the root path of the matnwb repository
6+
rootPath = misc.getMatnwbDir();
7+
8+
% Use a fixture to add the folder to the search path
9+
testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath));
10+
11+
% Use a fixture to create a temporary working directory
12+
testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture);
13+
generateCore('savedir', '.');
14+
end
15+
end
16+
17+
methods (Test)
18+
function testWarningIfAttributeDependencyMissing(testCase)
19+
% Test that warning is displayed when setting the value of a
20+
% property which depends on another property which is unset,
21+
% (typically an attribute of an untyped group or dataset)
22+
23+
imaging_plane = types.core.ImagingPlane( ...
24+
'device', types.untyped.SoftLink( types.core.Device() ), ...
25+
'excitation_lambda', 600., ...
26+
'indicator', 'GFP', ...
27+
'location', 'my favorite brain location');
28+
29+
% Same as "imaging_plane.grid_spacing_unit = 'micrometer'":
30+
testCase.verifyWarning(...
31+
@() setProperty(imaging_plane, 'grid_spacing_unit', 'micrometer'), ...
32+
'NWB:AttributeDependencyNotSet' ...
33+
)
34+
35+
function setProperty(nwbObject, propName, propValue)
36+
nwbObject.(propName) = propValue;
37+
end
38+
end
39+
end
40+
end

‎+tests/+unit/nwbExportTest.m

+31-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ function testExportNwbFileWithMissingRequiredAttribute(testCase)
4242
% This should raise an error because ProcessingModule requires the
4343
% 'description' property to be set (description is a required
4444
% attribute of ProcessingModule).
45+
4546
processingModule = types.core.ProcessingModule();
4647
testCase.NwbObject.processing.set('TestModule', processingModule);
4748

@@ -65,7 +66,7 @@ function testExportNwbFileWithMissingRequiredLink(testCase)
6566
testCase.verifyError(@(f, fn) nwbExport(testCase.NwbObject, nwbFilePath), ...
6667
'NWB:RequiredPropertyMissing')
6768

68-
% Clean up: the NwbObject is re-used by other tests.
69+
% Clean up: the NwbObject is reused by other tests.
6970
testCase.NwbObject.general_intracellular_ephys.remove('Electrode');
7071
end
7172

@@ -86,11 +87,40 @@ function testExportWithMissingRequiredDependentProperty(testCase)
8687

8788
% Verify that exporting the file issues warning that a required
8889
% property (i.e general_source_script_file_name) is missing
90+
8991
testCase.verifyWarning( ...
9092
@(nwbObj, filePath) nwbExport(nwbFile, fileName + "_2.nwb"), ...
9193
'NWB:DependentRequiredPropertyMissing')
9294
end
9395

96+
function testExportFileWithAttributeOfEmptyDataset(testCase)
97+
98+
nwbFile = testCase.initNwbFile();
99+
100+
% Add device to nwb object
101+
device = types.core.Device();
102+
nwbFile.general_devices.set('Device', device);
103+
104+
imaging_plane = types.core.ImagingPlane( ...
105+
'device', types.untyped.SoftLink(device), ...
106+
'excitation_lambda', 600., ...
107+
'indicator', 'GFP', ...
108+
'location', 'my favorite brain location');
109+
nwbFile.general_optophysiology.set('ImagingPlane', imaging_plane);
110+
111+
testCase.verifyWarningFree(...
112+
@() nwbExport(nwbFile, 'test_1.nwb'))
113+
114+
% Change value for attribute of the grid_spacing dataset.
115+
% Because grid_spacing is not set, this attribute value is not
116+
% exported to the file. Verify that warning is issued.
117+
imaging_plane.grid_spacing_unit = "microns";
118+
119+
testCase.verifyWarning(...
120+
@() nwbExport(nwbFile, 'test_2.nwb'), ...
121+
'NWB:DependentAttributeNotExported')
122+
end
123+
94124
function testExportTimeseriesWithMissingTimestampsAndStartingTime(testCase)
95125
time_series = types.core.TimeSeries( ...
96126
'data', linspace(0, 0.4, 50), ...

‎+types/+core/ElectricalSeries.m

+1-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
% READONLY PROPERTIES
99
properties(SetAccess = protected)
10-
channel_conversion_axis; % (int32) The zero-indexed axis of the 'data' dataset that the channel-specific conversion factor corresponds to. This value is fixed to 1.
10+
channel_conversion_axis = 1; % (int32) The zero-indexed axis of the 'data' dataset that the channel-specific conversion factor corresponds to. This value is fixed to 1.
1111
end
1212
% REQUIRED PROPERTIES
1313
properties
@@ -180,9 +180,6 @@
180180
if ~isempty(obj.channel_conversion) && ~isa(obj.channel_conversion, 'types.untyped.SoftLink') && ~isa(obj.channel_conversion, 'types.untyped.ExternalLink')
181181
io.writeAttribute(fid, [fullpath '/channel_conversion/axis'], obj.channel_conversion_axis);
182182
end
183-
if ~isempty(obj.channel_conversion) && isempty(obj.channel_conversion_axis)
184-
obj.warnIfRequiredDependencyMissing('channel_conversion_axis', 'channel_conversion', fullpath)
185-
end
186183
refs = obj.electrodes.export(fid, [fullpath '/electrodes'], refs);
187184
if ~isempty(obj.filtering)
188185
io.writeAttribute(fid, [fullpath '/filtering'], obj.filtering);

‎+types/+core/EventDetection.m

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
% READONLY PROPERTIES
99
properties(SetAccess = protected)
10-
times_unit; % (char) Unit of measurement for event times, which is fixed to 'seconds'.
10+
times_unit = "seconds"; % (char) Unit of measurement for event times, which is fixed to 'seconds'.
1111
end
1212
% REQUIRED PROPERTIES
1313
properties

‎+types/+core/ImageSeries.m

+7-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
dimension; % (int32) Number of pixels on x, y, (and z) axes.
1212
external_file; % (char) Paths to one or more external file(s). The field is only present if format='external'. This is only relevant if the image series is stored in the file system as one or more image file(s). This field should NOT be used if the image is stored in another NWB file and that file is linked to this file.
1313
external_file_starting_frame; % (int32) Each external image may contain one or more consecutive frames of the full ImageSeries. This attribute serves as an index to indicate which frames each file contains, to facilitate random access. The 'starting_frame' attribute, hence, contains a list of frame numbers within the full ImageSeries of the first frame of each file listed in the parent 'external_file' dataset. Zero-based indexing is used (hence, the first element will always be zero). For example, if the 'external_file' dataset has three paths to files and the first file has 5 frames, the second file has 10 frames, and the third file has 20 frames, then this attribute will have values [0, 5, 15]. If there is a single external file that holds all of the frames of the ImageSeries (and so there is a single element in the 'external_file' dataset), then this attribute should have value [0].
14-
format; % (char) Format of image. If this is 'external', then the attribute 'external_file' contains the path information to the image files. If this is 'raw', then the raw (single-channel) binary data is stored in the 'data' dataset. If this attribute is not present, then the default format='raw' case is assumed.
14+
format = "raw"; % (char) Format of image. If this is 'external', then the attribute 'external_file' contains the path information to the image files. If this is 'raw', then the raw (single-channel) binary data is stored in the 'data' dataset. If this attribute is not present, then the default format='raw' case is assumed.
1515
end
1616

1717
methods
@@ -100,6 +100,12 @@
100100
end
101101
function set.external_file_starting_frame(obj, val)
102102
obj.external_file_starting_frame = obj.validate_external_file_starting_frame(val);
103+
obj.postset_external_file_starting_frame()
104+
end
105+
function postset_external_file_starting_frame(obj)
106+
if isempty(obj.external_file) && ~isempty(obj.external_file_starting_frame)
107+
obj.warnIfAttributeDependencyMissing('external_file_starting_frame', 'external_file')
108+
end
103109
end
104110
function set.format(obj, val)
105111
obj.format = obj.validate_format(val);

0 commit comments

Comments
 (0)