Skip to content

Commit f89f9ec

Browse files
authored
Fix listing of required properties for neurodata types. (#688)
* Create functions for detecting property attributes * Update fillClass to correctly enumerate required properties in docstring * Move and fix function for getting required property names for class * Update getRequiredPropsForClass.m Resolve property attributes across the whole class hierarchy, and add some comments * Regenerate types * Update MetaClass to use updated function for retrieving required properties * Add option for providing a namespace to the getRequiredPropsForClass function * Minor adjustments to file.internal.isPropertyRequired * Add test that would fail before * Added some comments to test for more clarity * Update check in +types/+util/+dynamictable/checkConfig.m Check for existence of file instead of existence of class (class might be present in memory even if the file is removed due to generating a newer NWB version) * Create HasQuantity.m * Add HasQuantity as superclass to Group, Dataset and Link * Add schema and test class * Refactor specs and tests to not use scratch * Remove extra blank lines * Add specification and test to test that specification quantities are correctly parsed * Fix bug in HasQuantity * Update MetaClass.m Expose getRequiredMethods publicly (but keep hidden) * Add tests for invalid quantity specifications * Remove debug statement * Fix merge mistake for types.untyped.MetaClass
1 parent c22e7d6 commit f89f9ec

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+751
-82
lines changed

+file/+internal/getRequiredPropertyNames.m

Lines changed: 0 additions & 26 deletions
This file was deleted.

+file/+internal/isPropertyHidden.m

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
function result = isPropertyHidden(propertyInfo, className, namespace)
2+
% isPropertyHidden - Determine if a property is hidden
3+
4+
if isa(propertyInfo, 'file.Attribute') || isa(propertyInfo, 'file.Dataset')
5+
if strcmp(namespace.name, 'hdmf_common') ...
6+
&& strcmp(className, 'VectorData') ...
7+
&& any(strcmp(propertyInfo.name, {'unit', 'sampling_rate', 'resolution'}))
8+
result = true;
9+
else
10+
result = false;
11+
end
12+
else
13+
result = false;
14+
end
15+
end

+file/+internal/isPropertyReadonly.m

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
function result = isPropertyReadonly(propertyInfo)
2+
% isPropertyReadonly - Determine if a property is read-only
3+
if isa(propertyInfo, 'file.Attribute') || isa(propertyInfo, 'file.Dataset')
4+
result = propertyInfo.readonly;
5+
else
6+
result = false;
7+
end
8+
end

+file/+internal/isPropertyRequired.m

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
function result = isPropertyRequired(propInfo, fullPropertyName, allClassprops)
2+
% isPropertyRequired - Determine if a property is required
3+
4+
if ischar(propInfo) || isa(propInfo, 'containers.Map') || isstruct(propInfo)
5+
result = true;
6+
elseif isa(propInfo, 'file.interface.HasProps')
7+
isSubPropertyRequired = false(size(propInfo));
8+
for iSubProp = 1:length(propInfo)
9+
p = propInfo(iSubProp);
10+
isSubPropertyRequired(iSubProp) = p.required;
11+
end
12+
result = all(isSubPropertyRequired);
13+
elseif isa(propInfo, 'file.Attribute')
14+
if isempty(propInfo.dependent)
15+
result = propInfo.required;
16+
else
17+
result = resolveRequiredForDependentProp(propInfo, fullPropertyName, allClassprops);
18+
end
19+
elseif isa(propInfo, 'file.Link')
20+
result = propInfo.required;
21+
else
22+
result = false;
23+
end
24+
end
25+
26+
function tf = resolveRequiredForDependentProp(propInfo, propertyName, allProps)
27+
% resolveRequiredForDependentProp - If a dependent property is required,
28+
% whether it is required on object level also depends on whether it's parent
29+
% property is required.
30+
if ~propInfo.required
31+
tf = false;
32+
else % Check if parent is required
33+
parentName = strrep(propertyName, ['_' propInfo.name], '');
34+
parentInfo = allProps(parentName);
35+
tf = parentInfo.required;
36+
end
37+
end

+file/fillClass.m

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@
9797
'classdef ' name ' < ' depnm ' & ' classTag newline... %header, dependencies
9898
'% ' upper(name) ' - ' class.doc]; %name, docstr
9999

100-
allClassProps = file.internal.mergeProps(classprops, superClassProps);
101-
allRequiredPropertyNames = file.internal.getRequiredPropertyNames(allClassProps);
100+
fullClassName = strjoin({'types', misc.str2validName(namespace.name), name}, '.');
101+
allRequiredPropertyNames = schemes.internal.getRequiredPropsForClass(fullClassName, namespace);
102102
if isempty(allRequiredPropertyNames)
103103
allRequiredPropertyNames = {'None'};
104104
end
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
function requiredPropertyNames = getRequiredPropsForClass(fullClassName, namespace)
2+
% getRequiredPropsForClass - List required properties for a neurodata type / class
3+
4+
arguments
5+
fullClassName (1,1) string % E.g types.core.TimeSeries
6+
namespace schemes.Namespace = schemes.Namespace.empty
7+
end
8+
9+
% For the NwbFile class, we need to replace it with the generated
10+
% NWBFile superclass in order to retrieve schema information correctly
11+
if strcmp(fullClassName, 'NwbFile')
12+
superclassNames = string(superclasses(fullClassName));
13+
fullClassName = superclassNames(endsWith(superclassNames, 'NWBFile'));
14+
end
15+
16+
% Load cached namespace specifications
17+
classNameSplit = strsplit(fullClassName, '.');
18+
className = classNameSplit(end);
19+
if isempty(namespace)
20+
namespaceName = classNameSplit(find(strcmp(classNameSplit, 'types'))+1);
21+
namespaceName = strrep(namespaceName, '_', '-');
22+
namespace = schemes.loadNamespace(namespaceName);
23+
end
24+
25+
% Process/parse the namespace specifications to retrieve attributes for
26+
% the class properties.
27+
processedTypeMap = containers.Map;
28+
[processed, classprops, ~] = file.processClass(className, namespace, processedTypeMap);
29+
if ~isempty(processed)
30+
superClassProps = cell(1, numel(processed)-1);
31+
for iSuper = 2:numel(processed)
32+
[~, superClassProps{iSuper-1}, ~] = file.processClass(processed(iSuper).type, namespace, processedTypeMap);
33+
end
34+
classprops = file.internal.mergeProps(classprops, superClassProps);
35+
end
36+
37+
% Resolve the required properties. For the final list of required properties,
38+
% we ignore both hidden and read-only properties.
39+
allPropertieNames = keys(classprops);
40+
[isRequired, isReadOnly, isHidden] = deal( false(1, classprops.Count) );
41+
42+
for iProp = 1:length(allPropertieNames)
43+
propertyName = allPropertieNames{iProp};
44+
prop = classprops(propertyName);
45+
isRequired(iProp) = file.internal.isPropertyRequired(prop, propertyName, classprops);
46+
isReadOnly(iProp) = file.internal.isPropertyReadonly(prop);
47+
isHidden(iProp) = file.internal.isPropertyHidden(prop, className, namespace);
48+
end
49+
50+
requiredPropertyNames = allPropertieNames(isRequired & ~isReadOnly & ~isHidden);
51+
end

+tests/+factory/ElectrodeTable.m

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
function electrodeTable = ElectrodeTable(nwbFile)
2+
% ElectrodeRegion - Create electrode region
3+
4+
arguments
5+
nwbFile (1,1) NwbFile % A device is required
6+
end
7+
8+
device = types.core.Device;
9+
nwbFile.general_devices.set('testDevice', device);
10+
11+
electrodeTable = types.hdmf_common.DynamicTable(...
12+
'colnames', {'location', 'group', 'group_name', 'label'}, ...
13+
'description', 'all electrodes');
14+
nwbFile.general_extracellular_ephys_electrodes = electrodeTable;
15+
16+
electrodeGroup = types.core.ElectrodeGroup( ...
17+
'description', sprintf('electrode group'), ...
18+
'location', 'brain area', ...
19+
'device', types.untyped.SoftLink(device) ...
20+
);
21+
22+
nwbFile.general_extracellular_ephys.set('ElectrodeGroup', electrodeGroup);
23+
24+
electrodeTable.addRow( ...
25+
'location', 'unknown', ...
26+
'group', types.untyped.ObjectView(electrodeGroup), ...
27+
'group_name', 'test electrode group', ...
28+
'label', 'test electrode');
29+
end

0 commit comments

Comments
 (0)