From a9e280d10b4df9f70932314ba034eda678978e73 Mon Sep 17 00:00:00 2001 From: Lawrence Date: Fri, 18 Feb 2022 18:25:48 -0500 Subject: [PATCH 1/6] Minor refactor to support dynamicprops Set --- +types/+util/+dynamictable/addRawData.m | 5 +++-- +types/+util/checkSet.m | 5 ++--- NwbFile.m | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/+types/+util/+dynamictable/addRawData.m b/+types/+util/+dynamictable/addRawData.m index 33e45ec8..ad78988b 100644 --- a/+types/+util/+dynamictable/addRawData.m +++ b/+types/+util/+dynamictable/addRawData.m @@ -13,9 +13,9 @@ function addRawData(DynamicTable, column, data) if isprop(DynamicTable, column) Vector = DynamicTable.(column); elseif isprop(DynamicTable, 'vectorindex') && DynamicTable.vectorindex.isKey(column) - Vector = DynamicTable.vectorindex.get(column); + Vector = DynamicTable.vectorindex.(column); else - Vector = DynamicTable.vectordata.get(column); + Vector = DynamicTable.vectordata.(column); end % grab all available indices for column. @@ -81,6 +81,7 @@ function initVecData(DynamicTable, column) if isprop(DynamicTable, column) DynamicTable.(column) = VecData; else +% DynamicTable.vectordata.(column) = VecData; DynamicTable.vectordata.set(column, VecData); end end diff --git a/+types/+util/checkSet.m b/+types/+util/checkSet.m index 16e2192e..ceea7ac5 100644 --- a/+types/+util/checkSet.m +++ b/+types/+util/checkSet.m @@ -7,7 +7,6 @@ function checkSet(pname, namedprops, constraints, val) 'NWB:CheckSet:InvalidType',... 'Property `%s` must be a `types.untyped.Set`', pname); -val.setValidationFcn(... - @(nm, val)types.util.checkConstraint(pname, nm, namedprops, constraints, val)); -val.validateAll(); +val.internal_validationFunction = @(nm, val)types.util.checkConstraint(pname, nm, ... + namedprops, constraints, val); end \ No newline at end of file diff --git a/NwbFile.m b/NwbFile.m index 76c3c288..179c0f44 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -211,7 +211,7 @@ function embedSpecifications(~, fid) getProperty = @(x, prop) x.(prop); elseif isa(obj, 'types.untyped.Set') propertyNames = obj.keys(); - getProperty = @(x, prop) x.get(prop); + getProperty = @(x, prop)x(prop); elseif isa(obj, 'types.untyped.Anon') propertyNames = {obj.name}; getProperty = @(x, prop) x.value; From 80de3cb0a7959ff474653f4c36a24ebfa4ca2117 Mon Sep 17 00:00:00 2001 From: Lawrence Date: Fri, 18 Feb 2022 18:25:16 -0500 Subject: [PATCH 2/6] Rewrite Set to use dynamic properties --- +types/+untyped/Set.m | 356 +++++++++++++++++++++++++----------------- 1 file changed, 212 insertions(+), 144 deletions(-) diff --git a/+types/+untyped/Set.m b/+types/+untyped/Set.m index 3459ec73..cc4414cb 100644 --- a/+types/+untyped/Set.m +++ b/+types/+untyped/Set.m @@ -1,78 +1,93 @@ -classdef Set < handle & matlab.mixin.CustomDisplay - properties(SetAccess=protected) - map; %containers.Map - fcn; %validation function +classdef Set < dynamicprops & matlab.mixin.CustomDisplay + properties + internal_validationFunction function_handle = function_handle.empty(); % validation function end - + + properties (SetAccess = protected) + internal_dynamicPropertyToH5Name(:,2) cell; % cell string matrix where first column is (name) and second column is (hdf5 name) + end + + properties (Access = private) + internal_dynamicPropertiesMap; % containers.Map (name) -> (meta.DynamicProperty) + end + methods + %% Constructor function obj = Set(varargin) % obj = SET returns an empty set % obj = SET(field1,value1,...,fieldN,valueN) returns a set from key value pairs % obj = SET(src) can be a struct or map % obj = SET(__,fcn) adds a validation function from a handle - obj.map = containers.Map; - + + obj.internal_dynamicPropertiesMap = containers.Map(... + 'KeyType', 'char', ... + 'ValueType', 'any'); + if nargin == 0 return; end - - switch class(varargin{1}) - case 'function_handle' - obj.fcn = varargin{1}; - case {'struct', 'containers.Map'} - src = varargin{1}; - if isstruct(src) - srcFields = fieldnames(src); - for i=1:length(srcFields) - obj.map(srcFields{i}) = src.(srcFields{i}); - end - else - srcKeys = keys(src); - obj.set(srcKeys, values(src, srcKeys)); - end - - if nargin > 1 - assert(isa(varargin{2}, 'function_handle'),... - '`fcn` Expected a function_handle type'); - obj.fcn = varargin{2}; - end - case 'char' - if mod(length(varargin), 2) == 1 - assert(isa(varargin{end}, 'function_handle'),... - '`fcn` Expected a function_handle type'); - obj.fcn = varargin{end}; - varargin(end) = []; - end - assert(mod(length(varargin), 2) == 0,... - ['KeyWord Argument Count Mismatch. '... - 'Number of Keys do not match number of values']); - assert(iscellstr(varargin(1:2:end)),... - 'KeyWord Argument Error: Keys must be char'); - obj.map = containers.Map(varargin(1:2:end), varargin(2:2:end)); + + numSourceArguments = length(varargin); + if isa(varargin{end}, 'function_handle') + obj.internal_validationFunction = varargin{end}; + numSourceArguments = numSourceArguments - 1; + end + + if 1 == numSourceArguments + assert(isstruct(varargin{1}) || isa(varargin{1}, 'containers.Map'), ... + 'NWB:Untyped:Set:InvalidArguments', ... + 'Expected a struct or a containers.Map. Got %s', class(varargin{1})); + if isstruct(varargin{1}) + sourceMap = containers.Map(fieldnames(varargin{1}), struct2cell(varargin{1})); + else + sourceMap = varargin{1}; + end + else + assert(0 == mod(numSourceArguments, 2) ... + && iscellstr(varargin(1:2:numSourceArguments)), ... + 'NWB:Untyped:Set:InvalidArguemnts', ... + 'Expected keyword arguments'); + sourceMap = containers.Map(... + varargin(1:2:numSourceArguments), varargin(2:2:numSourceArguments)); + end + + sourceNames = sourceMap.keys(); + for iKey = 1:length(sourceNames) + name = sourceNames{iKey}; + obj.addProperty(name, sourceMap(name)); end end - - function tf = isKey(obj, name) - tf = isKey(obj.map, name); - end - - %return object's keys - function k = keys(obj) - k = keys(obj.map); - end - - %return values of backed map - function v = values(obj) - v = values(obj.map); + + %% validation function propagation + function set.internal_validationFunction(obj, val) + obj.internal_validationFunction = val; + + if ~isempty(obj.internal_validationFunction) + dynamicPropertyNames = keys(obj.internal_dynamicPropertiesMap); + for iPropNames = 1:length(dynamicPropertyNames) + propName = dynamicPropertyNames{iPropNames}; + try + obj.internal_validationFunction(propName, obj.(propName)); + catch ME + error('NWB:Untyped:Set:ValidationFunctionFailure', ... + ['Failed to set validation function with message:\n ' ... + '%s\n\nConsider passing a validation function which passes for all ' ... + 'current properties or remove `%s` from the Set.'], ... + ME.message, ... + propName); + end + end + end end - + + %% size() override %return number of entries function cnt = Count(obj) - cnt = obj.map.Count; + cnt = obj.internal_dynamicPropertiesMap.Count; end - - %overloads size(obj) + function varargout = size(obj, dim) + % overloads size(obj) if nargin > 1 if dim > 1 varargout{1} = 1; @@ -88,105 +103,134 @@ end end end - - %overloads horzcat(A1,A2,...,An) + function C = horzcat(varargin) + % overloads horzcat(A1,A2,...,An) error('NWB:Set:Unsupported',... 'types.untyped.Set does not support concatenation'); end - - %overloads vertcat(A1, A2,...,An) + function C = vertcat(varargin) + % overloads vertcat(A1, A2,...,An) error('NWB:Set:Unsupported',... 'types.untyped.Set does not support concatenation.'); end - - function setValidationFcn(obj, fcn) - if (~isnumeric(fcn) || ~isempty(fcn)) && ~isa(fcn, 'function_handle') - error('Validation must be a function handle of form @(name, val) or empty array.'); + + %% Add/Remove Properties + function addProperty(obj, name, value) + validateattributes(name, {'char'}, {'scalartext'}, 'addProperty', 'name', 1); + + fixedName = matlab.lang.makeValidName(name, 'Prefix', 'matnwb'); + assert(~obj.isH5Name(name) && ~obj.isPropName(fixedName), ... + 'NWB:Untyped:Set:DuplicateName', ... + 'The provided property name `%s` (converted to `%s`) is a duplicate name.', ... + name, fixedName); + height = size(obj.internal_dynamicPropertyToH5Name, 1); + obj.internal_dynamicPropertyToH5Name(height+1, 1:2) = {fixedName, name}; + obj.internal_dynamicPropertiesMap(fixedName) = obj.addprop(fixedName); + if ~isempty(obj.internal_validationFunction) + DynamicProperty = obj.internal_dynamicPropertiesMap(fixedName); + DynamicProperty.SetMethod = getDynamicSetMethodFilterFunction(fixedName); end - obj.fcn = fcn; + obj.(fixedName) = value; end - - function validateAll(obj) - mapkeys = keys(obj.map); - for i=1:length(mapkeys) - mk = mapkeys{i}; - obj.fcn(mk, obj.map(mk)); + + function value = removeProperty(obj, name) + validateattributes(name, {'char'}, {'scalartext'}, 'removeProperty', 'name', 1); + + assert(obj.isH5Name(name) || obj.isPropName(name), ... + 'NWB:Untyped:Set:MissingName', ... + 'Property name or HDF5 identifier `%s` does not exist for this Set.', ... + name); + + if obj.isH5Name(name) + name = obj.mapH5Name2PropertyName(name); end + + delete(obj.internal_dynamicPropertiesMap(name)); + remove(obj.internal_dynamicPropertiesMap, name); end - + + %% LEGACY GET/SET METHODS function obj = set(obj, name, val) - if ischar(name) - name = {name}; - end - - if ischar(val) - val = {val}; - end - cellExtract = iscell(val); - - assert(length(name) == length(val),... - 'number of property names should match number of vals on set.'); - if ~isempty(obj.fcn) - for i=1:length(name) - if cellExtract - elem = val{i}; - else - elem = val(i); - end - obj.fcn(name{i}, elem); - end + validateattributes(name, {'char'}, {'scalartext'}, 'set', 'name', 1); + + if ~obj.isH5Name(name) && ~obj.isPropName(name) + obj.addProperty(name, val); + return; end - for i=1:length(name) - if cellExtract - elem = val{i}; - else - elem = val(i); - end - obj.map(name{i}) = elem; + + if obj.isH5Name(name) + name = obj.mapH5Name2PropertyName(name); + end + + if isempty(val) + obj.removeProperty(name); + else + obj.(name) = val; end end - - function obj = remove(obj, name) - remove(obj.map, name); + + function o = get(obj, name) + if obj.isH5Name(name) + name = obj.mapH5Name2PropertyName(name); + end + assert(obj.isPropName(name), 'NWB:Untyped:Set:MissingName', ... + 'Could not find property name `%s`', name); + o = obj.(name); end - - function obj = clear(obj) - obj.map = containers.Map; + + %% LEGACY KEY METHODS + function propNames = keys(obj) + propNames = keys(obj.internal_dynamicPropertiesMap); end - - function o = get(obj, name) - if ischar(name) - name = {name}; + + function propValues = values(obj) + propValues = keys(obj); + for iProp = 1:length(propValues) + propName = propValues{iProp}; + propValues{iProp} = obj.(propName); end - o = cell(length(name),1); - for i=1:length(name) - o{i} = obj.map(name{i}); + end + + function remove(obj, keys) + if ischar(keys) + keys = {keys}; end - if isscalar(o) - o = o{1}; + assert(iscellstr(keys), 'NWB:Untyped:Set:InvalidArgument', ... + 'removed keys provided must be a cell array of strings.'); + for iKey = 1:length(keys) + obj.removeProperty(keys{iKey}); end end - + + function tf = isKey(obj, name) + tf = obj.isH5Name(name) || obj.isPropName(name); + end + + %% Export function refs = export(obj, fid, fullpath, refs) io.writeGroup(fid, fullpath); - k = keys(obj.map); - val = values(obj.map, k); - for i=1:length(k) - v = val{i}; - nm = k{i}; - propfp = [fullpath '/' nm]; - if startsWith(class(v), 'types.') - refs = v.export(fid, propfp, refs); + + dynamicPropertyNames = keys(obj.internal_dynamicPropertiesMap); + for iPropName = 1:length(dynamicPropertyNames) + propName = dynamicPropertyNames{iPropName}; + h5Name = obj.mapPropName2H5Name(propName); + propValue = obj.(propName); + + propFullPath = [fullpath '/' h5Name]; + if startsWith(class(propValue), 'types.') + refs = propValue.export(fid, propFullPath, refs); else - refs = io.writeDataset(fid, propfp, v, refs); + io.writeDataset(fid, propFullPath, propValue); end end end end - - methods(Access=protected) + + methods (Access = protected) + %% matlab.mixin.CustomDisplay overrides + function displayEmptyObject(obj) hdr = [' Empty '... ''... @@ -194,11 +238,11 @@ function displayEmptyObject(obj) footer = getFooter(obj); disp([hdr newline footer]); end - + function displayScalarObject(obj) displayNonScalarObject(obj) end - + function displayNonScalarObject(obj) hdr = getHeader(obj); footer = getFooter(obj); @@ -209,25 +253,49 @@ function displayNonScalarObject(obj) for i=1:length(mkeys) mk = mkeys{i}; mkspace = repmat(' ', 1, max_mklen - mklen(i)); - body{i} = [mkspace mk ': [' class(obj.map(mk)) ']']; + body{i} = [mkspace mk ': [' class(obj.(mk)) ']']; end body = file.addSpaces(strjoin(body, newline), 4); disp([hdr newline body newline footer]); end end - - methods(Access=private) - %converts to cell string. Does not do type checking. - function cellval = merge_stringtypes(obj, val) - if isstring(val) - val = convertStringsToChars(val); - end - - if ischar(val) - cellval = {val}; - else - cellval = val; - end + + methods (Access = private) + %% cell array table utilities + function tf = isPropName(obj, name) + validateattributes(name, {'char'}, {'scalartext'}, 'isPropName', 'name', 1); + tf = any(strcmp(obj.internal_dynamicPropertyToH5Name(:,1), name)); + end + + function tf = isH5Name(obj, name) + validateattributes(name, {'char'}, {'scalartext'}, 'isH5Name', 'name', 1); + tf = any(strcmp(obj.internal_dynamicPropertyToH5Name(:,2), name)); + end + + function propName = mapH5Name2PropertyName(obj, h5Name) + assert(obj.isH5Name(h5Name)); + rowIndex = find(strcmp(obj.internal_dynamicPropertyToH5Name(:,2), h5Name), 1); + propName = obj.internal_dynamicPropertyToH5Name{rowIndex,1}; + end + + function h5Name = mapPropName2H5Name(obj, propName) + assert(obj.isPropName(propName)); + rowIndex = find(strcmp(obj.internal_dynamicPropertyToH5Name(:,1), propName), 1); + h5Name = obj.internal_dynamicPropertyToH5Name{rowIndex,2}; + end + + + end +end + +function setterFunction = getDynamicSetMethodFilterFunction(name) +% workaround as provided by https://www.mathworks.com/matlabcentral/answers/266684-how-do-i-write-setter-methods-for-properties-with-unknown-names +setterFunction = @setProp; + + function setProp(obj, val) + if ~isempty(obj.internal_validationFunction) + obj.internal_validationFunction(name, val); end + obj.(name) = val; end end \ No newline at end of file From 3e1c6f88ab70117fe2f636c777783a6bb6a8416c Mon Sep 17 00:00:00 2001 From: Lawrence Date: Tue, 22 Feb 2022 10:05:32 -0500 Subject: [PATCH 3/6] Re-add `clear` to Set --- +types/+untyped/Set.m | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/+types/+untyped/Set.m b/+types/+untyped/Set.m index cc4414cb..218ec8fc 100644 --- a/+types/+untyped/Set.m +++ b/+types/+untyped/Set.m @@ -208,6 +208,14 @@ function remove(obj, keys) tf = obj.isH5Name(name) || obj.isPropName(name); end + function clear(obj) + DynamicProperties = obj.internal_dynamicPropertiesMap.values(); + for iProp = 1:length(DynamicProperties) + delete(DynamicProperties{iProp}); + end + remove(obj.internal_dynamicPropertiesMap, keys(obj.internal_dynamicPropertiesMap)); + end + %% Export function refs = export(obj, fid, fullpath, refs) io.writeGroup(fid, fullpath); From 52f77a20cb2a9244c1457306d6a7823209e633dc Mon Sep 17 00:00:00 2001 From: Lawrence Date: Tue, 22 Feb 2022 10:09:54 -0500 Subject: [PATCH 4/6] fix missing argout setting --- +types/+untyped/Set.m | 1 + 1 file changed, 1 insertion(+) diff --git a/+types/+untyped/Set.m b/+types/+untyped/Set.m index 218ec8fc..7146eb28 100644 --- a/+types/+untyped/Set.m +++ b/+types/+untyped/Set.m @@ -146,6 +146,7 @@ function addProperty(obj, name, value) if obj.isH5Name(name) name = obj.mapH5Name2PropertyName(name); end + value = obj.(name); delete(obj.internal_dynamicPropertiesMap(name)); remove(obj.internal_dynamicPropertiesMap, name); From 5cba5a4dabdcebca88619479e8f3476661e65cdf Mon Sep 17 00:00:00 2001 From: Lawrence Date: Tue, 22 Feb 2022 10:12:12 -0500 Subject: [PATCH 5/6] vastly simplify Set clear --- +types/+untyped/Set.m | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/+types/+untyped/Set.m b/+types/+untyped/Set.m index 7146eb28..0eb752cd 100644 --- a/+types/+untyped/Set.m +++ b/+types/+untyped/Set.m @@ -210,11 +210,7 @@ function remove(obj, keys) end function clear(obj) - DynamicProperties = obj.internal_dynamicPropertiesMap.values(); - for iProp = 1:length(DynamicProperties) - delete(DynamicProperties{iProp}); - end - remove(obj.internal_dynamicPropertiesMap, keys(obj.internal_dynamicPropertiesMap)); + obj.remove(keys(obj.internal_dynamicPropertiesMap)); end %% Export From ffd15337211d594e33154d8a760f51f8e6d48cc6 Mon Sep 17 00:00:00 2001 From: Lawrence Date: Tue, 22 Feb 2022 10:21:08 -0500 Subject: [PATCH 6/6] Fix NwbFile searchFor method for Sets --- NwbFile.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NwbFile.m b/NwbFile.m index 179c0f44..f462c175 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -211,7 +211,7 @@ function embedSpecifications(~, fid) getProperty = @(x, prop) x.(prop); elseif isa(obj, 'types.untyped.Set') propertyNames = obj.keys(); - getProperty = @(x, prop)x(prop); + getProperty = @(x, prop)x.(prop); elseif isa(obj, 'types.untyped.Anon') propertyNames = {obj.name}; getProperty = @(x, prop) x.value;