From 6f6e6c613b86ec7ad2563b10d528b64689859b3f Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 29 Mar 2026 20:46:59 +0200 Subject: [PATCH 1/7] Hoist typed dataset attributes onto parent classes --- +file/+internal/filterClassPropsForCodegen.m | 43 ++++++ +file/Attribute.m | 2 + +file/Group.m | 21 +++ +file/fillClass.m | 3 +- +file/fillExport.m | 19 ++- +file/fillSetters.m | 16 +- +io/parseDataset.m | 30 +++- +io/parseGroup.m | 4 +- +schemes/+internal/getRequiredPropsForClass.m | 1 + +tests/+system/UnitTimesIOTest.m | 12 +- +types/+core/Units.m | 139 ++++++++++++++++++ 11 files changed, 274 insertions(+), 16 deletions(-) create mode 100644 +file/+internal/filterClassPropsForCodegen.m diff --git a/+file/+internal/filterClassPropsForCodegen.m b/+file/+internal/filterClassPropsForCodegen.m new file mode 100644 index 000000000..accf60d2b --- /dev/null +++ b/+file/+internal/filterClassPropsForCodegen.m @@ -0,0 +1,43 @@ +function classprops = filterClassPropsForCodegen(classprops, namespace) +% filterClassPropsForCodegen - Remove redundant generated properties. + + classprops = removeRedundantTypedDatasetAttributeHoists(classprops, namespace); +end + +function classprops = removeRedundantTypedDatasetAttributeHoists(classprops, namespace) + propertyNames = keys(classprops); + toRemove = {}; + + for iProp = 1:length(propertyNames) + propertyName = propertyNames{iProp}; + prop = classprops(propertyName); + if ~isa(prop, 'file.Attribute') || isempty(prop.dependent) || ~prop.dependent_typed + continue; + end + + if ~isKey(classprops, prop.dependent) + continue; + end + + parentProp = classprops(prop.dependent); + if ~isa(parentProp, 'file.Dataset') || isempty(parentProp.type) + continue; + end + + childNamespace = namespace.getNamespace(parentProp.type); + if isempty(childNamespace) + continue; + end + + % Skip hoisting when the child typed dataset already exposes the + % attribute as part of its own public API. + isHiddenOnChild = file.internal.isPropertyHidden(prop, parentProp.type, childNamespace); + if ~isHiddenOnChild + toRemove{end+1} = propertyName; %#ok + end + end + + if ~isempty(toRemove) + remove(classprops, unique(toRemove)); + end +end diff --git a/+file/Attribute.m b/+file/Attribute.m index 90df92642..e35c380c1 100644 --- a/+file/Attribute.m +++ b/+file/Attribute.m @@ -8,6 +8,7 @@ dtype; %type of value dependent; %set externally. If the attribute is actually dependent on an untyped dataset/group 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. + dependent_typed = false; % set externally when hoisted from a typed dataset scalar; %if the value is scalar or an array dimnames; shape; @@ -25,6 +26,7 @@ obj.dtype = ''; obj.dependent = ''; obj.dependent_fullname = ''; + obj.dependent_typed = false; obj.scalar = true; obj.shape = {}; obj.dimnames = {}; diff --git a/+file/Group.m b/+file/Group.m index 68764dcba..669a61c81 100644 --- a/+file/Group.m +++ b/+file/Group.m @@ -162,6 +162,13 @@ end PropertyMap(SubData.name) = SubData; else + hoistedAttributes = getHoistedTypedDatasetAttributes(obj, SubData); + if ~isempty(hoistedAttributes) + attrNames = {hoistedAttributes.name}; + attrNames = strcat(SubData.name, '_', attrNames); + PropertyMap = [PropertyMap; ... + containers.Map(attrNames, num2cell(hoistedAttributes))]; + end if isempty(SubData.name) PropertyMap(lower(SubData.type)) = SubData; else @@ -253,3 +260,17 @@ end end end + +function hoistedAttributes = getHoistedTypedDatasetAttributes(GroupObj, datasetObj) + hoistedAttributes = file.Attribute.empty; + if isempty(GroupObj.type) || isempty(datasetObj.name) || isempty(datasetObj.attributes) + return; + end + + for iAttr = 1:length(datasetObj.attributes) + attribute = datasetObj.attributes(iAttr); + attribute.dependent = datasetObj.name; + attribute.dependent_typed = true; + hoistedAttributes(end+1) = attribute; %#ok + end +end diff --git a/+file/fillClass.m b/+file/fillClass.m index 41b55a4f6..138d91186 100644 --- a/+file/fillClass.m +++ b/+file/fillClass.m @@ -4,6 +4,7 @@ %% PROCESSING class = processed(1); + classprops = file.internal.filterClassPropsForCodegen(classprops, namespace); allProperties = keys(classprops); required = {}; @@ -165,7 +166,7 @@ inherited); setterFcns = file.fillSetters(setdiff(nonInherited, union(readonly, hiddenAndReadonly)), classprops); validatorFcns = file.fillValidators(allProperties, classprops, namespace, namespace.getFullClassName(name), inherited); - exporterFcns = file.fillExport(nonInherited, class, superclassNames{1}, required); + exporterFcns = file.fillExport(nonInherited, class, superclassNames{1}, required, classprops); methodBody = strjoin({constructorBody... '%% SETTERS' setterFcns... '%% VALIDATORS' validatorFcns... diff --git a/+file/fillExport.m b/+file/fillExport.m index 1a0412358..e198415cc 100644 --- a/+file/fillExport.m +++ b/+file/fillExport.m @@ -1,4 +1,4 @@ -function festr = fillExport(propertyNames, RawClass, parentName, required) +function festr = fillExport(propertyNames, RawClass, parentName, required, classprops) exportHeader = 'function refs = export(obj, fid, fullpath, refs)'; if isa(RawClass, 'file.Dataset') propertyNames = propertyNames(~strcmp(propertyNames, 'data')); @@ -22,6 +22,11 @@ propertyName = propertyNames{i}; pathProps = traverseRaw(propertyName, RawClass); prop = pathProps{end}; + if nargin >= 5 && isa(prop, 'file.Attribute') ... + && isKey(classprops, propertyName) ... + && isa(classprops(propertyName), 'file.Attribute') + prop = classprops(propertyName); + end elideProps = pathProps(1:end-1); elisions = cell(length(elideProps),1); % Construct elisions @@ -222,6 +227,7 @@ propertyChecks = {}; dependencyCheck = {}; + preExportString = ''; if isa(prop, 'file.Attribute') && ~isempty(prop.dependent) %if attribute is dependent, check before writing @@ -254,6 +260,13 @@ warnIfMissingRequiredDependentAttributeStr = ... sprintf('obj.throwErrorIfRequiredDependencyMissing(''%s'', ''%s'', fullpath)', name, depPropname); end + + if prop.dependent_typed + preExportString = sprintf([ ... + 'if isempty(obj.%1$s) && ~isempty(obj.%2$s) && isobject(obj.%2$s) && isprop(obj.%2$s, ''%3$s'') && ~isempty(obj.%2$s.%3$s)\n' ... + ' obj.%1$s = obj.%2$s.%3$s;\n' ... + 'end'], name, depPropname, prop.name); + end end if ~prop.required @@ -273,6 +286,10 @@ end end + if ~isempty(preExportString) + dataExportString = sprintf('%s\n%s', preExportString, dataExportString); + end + if ~isempty(dependencyCheck) dataExportString = sprintf('%s\nif %s\n%s\nend', ... dataExportString, ... diff --git a/+file/fillSetters.m b/+file/fillSetters.m index 169bf4292..379a2ad92 100644 --- a/+file/fillSetters.m +++ b/+file/fillSetters.m @@ -43,13 +43,25 @@ warnIfDependencyMissingString = sprintf(... 'obj.warnIfAttributeDependencyMissing(''%s'', ''%s'')', ... propname, parentname); + + syncTypedDatasetAttributeString = ''; + if prop.dependent_typed + syncTypedDatasetAttributeString = sprintf([ ... + 'if ~isempty(obj.%1$s) && isobject(obj.%1$s) && isprop(obj.%1$s, ''%2$s'')\n' ... + ' obj.%1$s.%2$s = obj.%3$s;\n' ... + 'end'], parentname, prop.name, propname); + end - postsetFunctionStr = strjoin({... + postsetLines = {... sprintf('function postset_%s(obj)', propname), ... file.addSpaces(conditionStr, 4), ... file.addSpaces(warnIfDependencyMissingString, 8), ... file.addSpaces('end', 4), ... - 'end'}, newline); + 'end'}; + if ~isempty(syncTypedDatasetAttributeString) + postsetLines = [postsetLines(1:end-1), {file.addSpaces(syncTypedDatasetAttributeString, 4)}, postsetLines(end)]; + end + postsetFunctionStr = strjoin(postsetLines, newline); end end end diff --git a/+io/parseDataset.m b/+io/parseDataset.m index 62a1342be..27ddfab17 100644 --- a/+io/parseDataset.m +++ b/+io/parseDataset.m @@ -1,7 +1,10 @@ -function parsed = parseDataset(filename, info, fullpath, Blacklist) +function parsed = parseDataset(filename, info, fullpath, Blacklist, parentTypeName) %typed and untyped being container maps containing type and untyped datasets % the maps store information regarding information and stored data % NOTE, dataset name is in path format so we need to parse that out. + if nargin < 5 + parentTypeName = ''; + end name = info.Name; %check if typed and parse attributes @@ -16,8 +19,15 @@ parsed = containers.Map; afields = keys(attrargs); if ~isempty(afields) - anames = strcat(name, '_', afields); - parsed = [parsed; containers.Map(anames, attrargs.values(afields))]; + hoistedFields = afields; + if ~isempty(Type.typename) && ~isempty(parentTypeName) + hoistedFields = filterHoistedFieldsForParent(parentTypeName, name, afields); + end + + if ~isempty(hoistedFields) + anames = strcat(name, '_', hoistedFields); + parsed = [parsed; containers.Map(anames, attrargs.values(hoistedFields))]; + end end % loading h5t references are required @@ -83,8 +93,20 @@ else props('data') = data; kwargs = io.map2kwargs(props); - parsed = io.createParsedType(fullpath, Type.typename, kwargs{:}); + parsed(name) = io.createParsedType(fullpath, Type.typename, kwargs{:}); end H5D.close(did); H5F.close(fid); end + +function hoistedFields = filterHoistedFieldsForParent(parentTypeName, datasetName, availableFields) + metaClass = meta.class.fromName(parentTypeName); + if isempty(metaClass) + hoistedFields = availableFields; + return; + end + + parentPropertyNames = {metaClass.PropertyList.Name}; + prefixedFieldNames = strcat(datasetName, '_', availableFields); + hoistedFields = availableFields(ismember(prefixedFieldNames, parentPropertyNames)); +end diff --git a/+io/parseGroup.m b/+io/parseGroup.m index 70122dd1a..6dc930d50 100644 --- a/+io/parseGroup.m +++ b/+io/parseGroup.m @@ -19,7 +19,7 @@ for i=1:length(info.Datasets) datasetInfo = info.Datasets(i); fullPath = [info.Name '/' datasetInfo.Name]; - dataset = io.parseDataset(filename, datasetInfo, fullPath, Blacklist); + dataset = io.parseDataset(filename, datasetInfo, fullPath, Blacklist, Type.typename); if isa(dataset, 'containers.Map') datasetProperties = [datasetProperties; dataset]; else @@ -137,4 +137,4 @@ end end remove(set, elidekeys(drop)); %delete all leftovers that were yielded -end \ No newline at end of file +end diff --git a/+schemes/+internal/getRequiredPropsForClass.m b/+schemes/+internal/getRequiredPropsForClass.m index 5d6111834..da9fc9ea5 100644 --- a/+schemes/+internal/getRequiredPropsForClass.m +++ b/+schemes/+internal/getRequiredPropsForClass.m @@ -33,6 +33,7 @@ end classprops = file.internal.mergeProps(classprops, superClassProps); end + classprops = file.internal.filterClassPropsForCodegen(classprops, namespace); % Resolve the required properties. For the final list of required properties, % we ignore both hidden and read-only properties. diff --git a/+tests/+system/UnitTimesIOTest.m b/+tests/+system/UnitTimesIOTest.m index a5ba844e0..62ec51d9e 100644 --- a/+tests/+system/UnitTimesIOTest.m +++ b/+tests/+system/UnitTimesIOTest.m @@ -40,12 +40,12 @@ function addContainer(~, file) , 'data', 1 ... ); - % set optional hidden vector data attributes - file.units.spike_times.resolution = 3; + % set optional Units table dataset attributes via hoisted API + file.units.spike_times_resolution = 3; Units = file.units; - [Units.waveform_mean.sampling_rate ... - , Units.waveform_sd.sampling_rate ... - , Units.waveforms.sampling_rate ... + [Units.waveform_mean_sampling_rate ... + , Units.waveform_sd_sampling_rate ... + , Units.waveforms_sampling_rate ... ] = deal(1); end @@ -53,4 +53,4 @@ function addContainer(~, file) c = file.units; end end -end \ No newline at end of file +end diff --git a/+types/+core/Units.m b/+types/+core/Units.m index fb32fb3ac..e4bd64dc3 100644 --- a/+types/+core/Units.m +++ b/+types/+core/Units.m @@ -5,6 +5,12 @@ % colnames, description, id +% READONLY PROPERTIES +properties(SetAccess = protected) + waveform_mean_unit = "volts"; % (char) Unit of measurement. This value is fixed to 'volts'. + waveform_sd_unit = "volts"; % (char) Unit of measurement. This value is fixed to 'volts'. + waveforms_unit = "volts"; % (char) Unit of measurement. This value is fixed to 'volts'. +end % OPTIONAL PROPERTIES properties electrode_group; % (VectorData) Electrode group that each spike unit came from. @@ -14,11 +20,15 @@ obs_intervals_index; % (VectorIndex) Index into the obs_intervals dataset. spike_times; % (VectorData) Spike times for each unit in seconds. spike_times_index; % (VectorIndex) Index into the spike_times dataset. + spike_times_resolution; % (double) The smallest possible difference between two spike times. Usually 1 divided by the acquisition sampling rate from which spike times were extracted, but could be larger if the acquisition time series was downsampled or smaller if the acquisition time series was smoothed/interpolated and it is possible for the spike time to be between samples. waveform_mean; % (VectorData) Spike waveform mean for each spike unit. + waveform_mean_sampling_rate; % (single) Sampling rate, in hertz. waveform_sd; % (VectorData) Spike waveform standard deviation for each spike unit. + waveform_sd_sampling_rate; % (single) Sampling rate, in hertz. waveforms; % (VectorData) Individual waveforms for each spike on each electrode. This is a doubly indexed column. The 'waveforms_index' column indexes which waveforms in this column belong to the same spike event for a given unit, where each waveform was recorded from a different electrode. The 'waveforms_index_index' column indexes the 'waveforms_index' column to indicate which spike events belong to a given unit. For example, if the 'waveforms_index_index' column has values [2, 5, 6], then the first 2 elements of the 'waveforms_index' column correspond to the 2 spike events of the first unit, the next 3 elements of the 'waveforms_index' column correspond to the 3 spike events of the second unit, and the next 1 element of the 'waveforms_index' column corresponds to the 1 spike event of the third unit. If the 'waveforms_index' column has values [3, 6, 8, 10, 12, 13], then the first 3 elements of the 'waveforms' column contain the 3 spike waveforms that were recorded from 3 different electrodes for the first spike time of the first unit. See https://nwb-schema.readthedocs.io/en/stable/format_description.html#doubly-ragged-arrays for a graphical representation of this example. When there is only one electrode for each unit (i.e., each spike time is associated with a single waveform), then the 'waveforms_index' column will have values 1, 2, ..., N, where N is the number of spike events. The number of electrodes for each spike event should be the same within a given unit. The 'electrodes' column should be used to indicate which electrodes are associated with each unit, and the order of the waveforms within a given unit x spike event should be the same as the order of the electrodes referenced in the 'electrodes' column of this table. The number of samples for each waveform must be the same. waveforms_index; % (VectorIndex) Index into the 'waveforms' dataset. One value for every spike event. See 'waveforms' for more detail. waveforms_index_index; % (VectorIndex) Index into the 'waveforms_index' dataset. One value for every unit (row in the table). See 'waveforms' for more detail. + waveforms_sampling_rate; % (single) Sampling rate, in hertz. end methods @@ -51,21 +61,30 @@ % % - spike_times_index (VectorIndex) - Index into the spike_times dataset. % + % - spike_times_resolution (double) - The smallest possible difference between two spike times. Usually 1 divided by the acquisition sampling rate from which spike times were extracted, but could be larger if the acquisition time series was downsampled or smaller if the acquisition time series was smoothed/interpolated and it is possible for the spike time to be between samples. + % % - vectordata (VectorData) - Vector columns, including index columns, of this dynamic table. % % - waveform_mean (VectorData) - Spike waveform mean for each spike unit. % + % - waveform_mean_sampling_rate (single) - Sampling rate, in hertz. + % % - waveform_sd (VectorData) - Spike waveform standard deviation for each spike unit. % + % - waveform_sd_sampling_rate (single) - Sampling rate, in hertz. + % % - waveforms (VectorData) - Individual waveforms for each spike on each electrode. This is a doubly indexed column. The 'waveforms_index' column indexes which waveforms in this column belong to the same spike event for a given unit, where each waveform was recorded from a different electrode. The 'waveforms_index_index' column indexes the 'waveforms_index' column to indicate which spike events belong to a given unit. For example, if the 'waveforms_index_index' column has values [2, 5, 6], then the first 2 elements of the 'waveforms_index' column correspond to the 2 spike events of the first unit, the next 3 elements of the 'waveforms_index' column correspond to the 3 spike events of the second unit, and the next 1 element of the 'waveforms_index' column corresponds to the 1 spike event of the third unit. If the 'waveforms_index' column has values [3, 6, 8, 10, 12, 13], then the first 3 elements of the 'waveforms' column contain the 3 spike waveforms that were recorded from 3 different electrodes for the first spike time of the first unit. See https://nwb-schema.readthedocs.io/en/stable/format_description.html#doubly-ragged-arrays for a graphical representation of this example. When there is only one electrode for each unit (i.e., each spike time is associated with a single waveform), then the 'waveforms_index' column will have values 1, 2, ..., N, where N is the number of spike events. The number of electrodes for each spike event should be the same within a given unit. The 'electrodes' column should be used to indicate which electrodes are associated with each unit, and the order of the waveforms within a given unit x spike event should be the same as the order of the electrodes referenced in the 'electrodes' column of this table. The number of samples for each waveform must be the same. % % - waveforms_index (VectorIndex) - Index into the 'waveforms' dataset. One value for every spike event. See 'waveforms' for more detail. % % - waveforms_index_index (VectorIndex) - Index into the 'waveforms_index' dataset. One value for every unit (row in the table). See 'waveforms' for more detail. % + % - waveforms_sampling_rate (single) - Sampling rate, in hertz. + % % Output Arguments: % - units (types.core.Units) - A Units object + varargin = [{'waveform_mean_unit' 'volts' 'waveform_sd_unit' 'volts' 'waveforms_unit' 'volts'} varargin]; obj = obj@types.hdmf_common.DynamicTable(varargin{:}); @@ -80,11 +99,18 @@ addParameter(p, 'obs_intervals_index',[]); addParameter(p, 'spike_times',[]); addParameter(p, 'spike_times_index',[]); + addParameter(p, 'spike_times_resolution',[]); addParameter(p, 'waveform_mean',[]); + addParameter(p, 'waveform_mean_sampling_rate',[]); + addParameter(p, 'waveform_mean_unit',[]); addParameter(p, 'waveform_sd',[]); + addParameter(p, 'waveform_sd_sampling_rate',[]); + addParameter(p, 'waveform_sd_unit',[]); addParameter(p, 'waveforms',[]); addParameter(p, 'waveforms_index',[]); addParameter(p, 'waveforms_index_index',[]); + addParameter(p, 'waveforms_sampling_rate',[]); + addParameter(p, 'waveforms_unit',[]); misc.parseSkipInvalidName(p, varargin); obj.electrode_group = p.Results.electrode_group; obj.electrodes = p.Results.electrodes; @@ -93,11 +119,18 @@ obj.obs_intervals_index = p.Results.obs_intervals_index; obj.spike_times = p.Results.spike_times; obj.spike_times_index = p.Results.spike_times_index; + obj.spike_times_resolution = p.Results.spike_times_resolution; obj.waveform_mean = p.Results.waveform_mean; + obj.waveform_mean_sampling_rate = p.Results.waveform_mean_sampling_rate; + obj.waveform_mean_unit = p.Results.waveform_mean_unit; obj.waveform_sd = p.Results.waveform_sd; + obj.waveform_sd_sampling_rate = p.Results.waveform_sd_sampling_rate; + obj.waveform_sd_unit = p.Results.waveform_sd_unit; obj.waveforms = p.Results.waveforms; obj.waveforms_index = p.Results.waveforms_index; obj.waveforms_index_index = p.Results.waveforms_index_index; + obj.waveforms_sampling_rate = p.Results.waveforms_sampling_rate; + obj.waveforms_unit = p.Results.waveforms_unit; if strcmp(class(obj), 'types.core.Units') cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); @@ -128,12 +161,48 @@ function set.spike_times_index(obj, val) obj.spike_times_index = obj.validate_spike_times_index(val); end + function set.spike_times_resolution(obj, val) + obj.spike_times_resolution = obj.validate_spike_times_resolution(val); + obj.postset_spike_times_resolution() + end + function postset_spike_times_resolution(obj) + if isempty(obj.spike_times) && ~isempty(obj.spike_times_resolution) + obj.warnIfAttributeDependencyMissing('spike_times_resolution', 'spike_times') + end + if ~isempty(obj.spike_times) && isobject(obj.spike_times) && isprop(obj.spike_times, 'resolution') + obj.spike_times.resolution = obj.spike_times_resolution; + end + end function set.waveform_mean(obj, val) obj.waveform_mean = obj.validate_waveform_mean(val); end + function set.waveform_mean_sampling_rate(obj, val) + obj.waveform_mean_sampling_rate = obj.validate_waveform_mean_sampling_rate(val); + obj.postset_waveform_mean_sampling_rate() + end + function postset_waveform_mean_sampling_rate(obj) + if isempty(obj.waveform_mean) && ~isempty(obj.waveform_mean_sampling_rate) + obj.warnIfAttributeDependencyMissing('waveform_mean_sampling_rate', 'waveform_mean') + end + if ~isempty(obj.waveform_mean) && isobject(obj.waveform_mean) && isprop(obj.waveform_mean, 'sampling_rate') + obj.waveform_mean.sampling_rate = obj.waveform_mean_sampling_rate; + end + end function set.waveform_sd(obj, val) obj.waveform_sd = obj.validate_waveform_sd(val); end + function set.waveform_sd_sampling_rate(obj, val) + obj.waveform_sd_sampling_rate = obj.validate_waveform_sd_sampling_rate(val); + obj.postset_waveform_sd_sampling_rate() + end + function postset_waveform_sd_sampling_rate(obj) + if isempty(obj.waveform_sd) && ~isempty(obj.waveform_sd_sampling_rate) + obj.warnIfAttributeDependencyMissing('waveform_sd_sampling_rate', 'waveform_sd') + end + if ~isempty(obj.waveform_sd) && isobject(obj.waveform_sd) && isprop(obj.waveform_sd, 'sampling_rate') + obj.waveform_sd.sampling_rate = obj.waveform_sd_sampling_rate; + end + end function set.waveforms(obj, val) obj.waveforms = obj.validate_waveforms(val); end @@ -143,6 +212,18 @@ function set.waveforms_index_index(obj, val) obj.waveforms_index_index = obj.validate_waveforms_index_index(val); end + function set.waveforms_sampling_rate(obj, val) + obj.waveforms_sampling_rate = obj.validate_waveforms_sampling_rate(val); + obj.postset_waveforms_sampling_rate() + end + function postset_waveforms_sampling_rate(obj) + if isempty(obj.waveforms) && ~isempty(obj.waveforms_sampling_rate) + obj.warnIfAttributeDependencyMissing('waveforms_sampling_rate', 'waveforms') + end + if ~isempty(obj.waveforms) && isobject(obj.waveforms) && isprop(obj.waveforms, 'sampling_rate') + obj.waveforms.sampling_rate = obj.waveforms_sampling_rate; + end + end %% VALIDATORS function val = validate_electrode_group(obj, val) @@ -183,6 +264,10 @@ function val = validate_spike_times_index(obj, val) types.util.checkType('spike_times_index', 'types.hdmf_common.VectorIndex', val); end + function val = validate_spike_times_resolution(obj, val) + val = types.util.checkDtype('spike_times_resolution', 'double', val); + types.util.validateShape('spike_times_resolution', {[1]}, val) + end function val = validate_waveform_mean(obj, val) types.util.checkType('waveform_mean', 'types.hdmf_common.VectorData', val); if ~isempty(val) @@ -192,6 +277,10 @@ val = types.util.rewrapValue(val, originalVal); end end + function val = validate_waveform_mean_sampling_rate(obj, val) + val = types.util.checkDtype('waveform_mean_sampling_rate', 'single', val); + types.util.validateShape('waveform_mean_sampling_rate', {[1]}, val) + end function val = validate_waveform_sd(obj, val) types.util.checkType('waveform_sd', 'types.hdmf_common.VectorData', val); if ~isempty(val) @@ -201,6 +290,10 @@ val = types.util.rewrapValue(val, originalVal); end end + function val = validate_waveform_sd_sampling_rate(obj, val) + val = types.util.checkDtype('waveform_sd_sampling_rate', 'single', val); + types.util.validateShape('waveform_sd_sampling_rate', {[1]}, val) + end function val = validate_waveforms(obj, val) types.util.checkType('waveforms', 'types.hdmf_common.VectorData', val); if ~isempty(val) @@ -216,6 +309,10 @@ function val = validate_waveforms_index_index(obj, val) types.util.checkType('waveforms_index_index', 'types.hdmf_common.VectorIndex', val); end + function val = validate_waveforms_sampling_rate(obj, val) + val = types.util.checkDtype('waveforms_sampling_rate', 'single', val); + types.util.validateShape('waveforms_sampling_rate', {[1]}, val) + end %% EXPORT function refs = export(obj, fid, fullpath, refs) refs = export@types.hdmf_common.DynamicTable(obj, fid, fullpath, refs); @@ -243,12 +340,42 @@ if ~isempty(obj.spike_times_index) refs = obj.spike_times_index.export(fid, [fullpath '/spike_times_index'], refs); end + if isempty(obj.spike_times_resolution) && ~isempty(obj.spike_times) && isobject(obj.spike_times) && isprop(obj.spike_times, 'resolution') && ~isempty(obj.spike_times.resolution) + obj.spike_times_resolution = obj.spike_times.resolution; + end + if ~isempty(obj.spike_times) && ~isa(obj.spike_times, 'types.untyped.SoftLink') && ~isa(obj.spike_times, 'types.untyped.ExternalLink') && ~isempty(obj.spike_times_resolution) + io.writeAttribute(fid, [fullpath '/spike_times/resolution'], obj.spike_times_resolution); + end if ~isempty(obj.waveform_mean) refs = obj.waveform_mean.export(fid, [fullpath '/waveform_mean'], refs); end + if isempty(obj.waveform_mean_sampling_rate) && ~isempty(obj.waveform_mean) && isobject(obj.waveform_mean) && isprop(obj.waveform_mean, 'sampling_rate') && ~isempty(obj.waveform_mean.sampling_rate) + obj.waveform_mean_sampling_rate = obj.waveform_mean.sampling_rate; + end + if ~isempty(obj.waveform_mean) && ~isa(obj.waveform_mean, 'types.untyped.SoftLink') && ~isa(obj.waveform_mean, 'types.untyped.ExternalLink') && ~isempty(obj.waveform_mean_sampling_rate) + io.writeAttribute(fid, [fullpath '/waveform_mean/sampling_rate'], obj.waveform_mean_sampling_rate); + end + if isempty(obj.waveform_mean_unit) && ~isempty(obj.waveform_mean) && isobject(obj.waveform_mean) && isprop(obj.waveform_mean, 'unit') && ~isempty(obj.waveform_mean.unit) + obj.waveform_mean_unit = obj.waveform_mean.unit; + end + if ~isempty(obj.waveform_mean) && ~isa(obj.waveform_mean, 'types.untyped.SoftLink') && ~isa(obj.waveform_mean, 'types.untyped.ExternalLink') && ~isempty(obj.waveform_mean_unit) + io.writeAttribute(fid, [fullpath '/waveform_mean/unit'], obj.waveform_mean_unit); + end if ~isempty(obj.waveform_sd) refs = obj.waveform_sd.export(fid, [fullpath '/waveform_sd'], refs); end + if isempty(obj.waveform_sd_sampling_rate) && ~isempty(obj.waveform_sd) && isobject(obj.waveform_sd) && isprop(obj.waveform_sd, 'sampling_rate') && ~isempty(obj.waveform_sd.sampling_rate) + obj.waveform_sd_sampling_rate = obj.waveform_sd.sampling_rate; + end + if ~isempty(obj.waveform_sd) && ~isa(obj.waveform_sd, 'types.untyped.SoftLink') && ~isa(obj.waveform_sd, 'types.untyped.ExternalLink') && ~isempty(obj.waveform_sd_sampling_rate) + io.writeAttribute(fid, [fullpath '/waveform_sd/sampling_rate'], obj.waveform_sd_sampling_rate); + end + if isempty(obj.waveform_sd_unit) && ~isempty(obj.waveform_sd) && isobject(obj.waveform_sd) && isprop(obj.waveform_sd, 'unit') && ~isempty(obj.waveform_sd.unit) + obj.waveform_sd_unit = obj.waveform_sd.unit; + end + if ~isempty(obj.waveform_sd) && ~isa(obj.waveform_sd, 'types.untyped.SoftLink') && ~isa(obj.waveform_sd, 'types.untyped.ExternalLink') && ~isempty(obj.waveform_sd_unit) + io.writeAttribute(fid, [fullpath '/waveform_sd/unit'], obj.waveform_sd_unit); + end if ~isempty(obj.waveforms) refs = obj.waveforms.export(fid, [fullpath '/waveforms'], refs); end @@ -258,6 +385,18 @@ if ~isempty(obj.waveforms_index_index) refs = obj.waveforms_index_index.export(fid, [fullpath '/waveforms_index_index'], refs); end + if isempty(obj.waveforms_sampling_rate) && ~isempty(obj.waveforms) && isobject(obj.waveforms) && isprop(obj.waveforms, 'sampling_rate') && ~isempty(obj.waveforms.sampling_rate) + obj.waveforms_sampling_rate = obj.waveforms.sampling_rate; + end + if ~isempty(obj.waveforms) && ~isa(obj.waveforms, 'types.untyped.SoftLink') && ~isa(obj.waveforms, 'types.untyped.ExternalLink') && ~isempty(obj.waveforms_sampling_rate) + io.writeAttribute(fid, [fullpath '/waveforms/sampling_rate'], obj.waveforms_sampling_rate); + end + if isempty(obj.waveforms_unit) && ~isempty(obj.waveforms) && isobject(obj.waveforms) && isprop(obj.waveforms, 'unit') && ~isempty(obj.waveforms.unit) + obj.waveforms_unit = obj.waveforms.unit; + end + if ~isempty(obj.waveforms) && ~isa(obj.waveforms, 'types.untyped.SoftLink') && ~isa(obj.waveforms, 'types.untyped.ExternalLink') && ~isempty(obj.waveforms_unit) + io.writeAttribute(fid, [fullpath '/waveforms/unit'], obj.waveforms_unit); + end end end From b73a47252e2cd23a87a4235918dfd616896e6f01 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 29 Mar 2026 22:31:55 +0200 Subject: [PATCH 2/7] Use schema-defined attrs for included dataset promotion --- +file/+internal/filterClassPropsForCodegen.m | 43 ---------- +file/Attribute.m | 4 +- +file/Group.m | 29 ++++--- +file/fillClass.m | 2 +- +file/fillConstructor.m | 4 +- +file/fillExport.m | 2 +- +file/fillSetters.m | 10 +-- +file/processClass.m | 78 ++++++++++++++++++- +io/parseDataset.m | 26 +++---- +schemes/+internal/getRequiredPropsForClass.m | 2 - +tests/+system/UnitTimesIOTest.m | 2 +- 11 files changed, 121 insertions(+), 81 deletions(-) delete mode 100644 +file/+internal/filterClassPropsForCodegen.m diff --git a/+file/+internal/filterClassPropsForCodegen.m b/+file/+internal/filterClassPropsForCodegen.m deleted file mode 100644 index accf60d2b..000000000 --- a/+file/+internal/filterClassPropsForCodegen.m +++ /dev/null @@ -1,43 +0,0 @@ -function classprops = filterClassPropsForCodegen(classprops, namespace) -% filterClassPropsForCodegen - Remove redundant generated properties. - - classprops = removeRedundantTypedDatasetAttributeHoists(classprops, namespace); -end - -function classprops = removeRedundantTypedDatasetAttributeHoists(classprops, namespace) - propertyNames = keys(classprops); - toRemove = {}; - - for iProp = 1:length(propertyNames) - propertyName = propertyNames{iProp}; - prop = classprops(propertyName); - if ~isa(prop, 'file.Attribute') || isempty(prop.dependent) || ~prop.dependent_typed - continue; - end - - if ~isKey(classprops, prop.dependent) - continue; - end - - parentProp = classprops(prop.dependent); - if ~isa(parentProp, 'file.Dataset') || isempty(parentProp.type) - continue; - end - - childNamespace = namespace.getNamespace(parentProp.type); - if isempty(childNamespace) - continue; - end - - % Skip hoisting when the child typed dataset already exposes the - % attribute as part of its own public API. - isHiddenOnChild = file.internal.isPropertyHidden(prop, parentProp.type, childNamespace); - if ~isHiddenOnChild - toRemove{end+1} = propertyName; %#ok - end - end - - if ~isempty(toRemove) - remove(classprops, unique(toRemove)); - end -end diff --git a/+file/Attribute.m b/+file/Attribute.m index e35c380c1..2319daafc 100644 --- a/+file/Attribute.m +++ b/+file/Attribute.m @@ -8,7 +8,7 @@ dtype; %type of value dependent; %set externally. If the attribute is actually dependent on an untyped dataset/group 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. - dependent_typed = false; % set externally when hoisted from a typed dataset + promoted_to_container = false; % set externally when promoted from a typed dataset onto the containing class API scalar; %if the value is scalar or an array dimnames; shape; @@ -26,7 +26,7 @@ obj.dtype = ''; obj.dependent = ''; obj.dependent_fullname = ''; - obj.dependent_typed = false; + obj.promoted_to_container = false; obj.scalar = true; obj.shape = {}; obj.dimnames = {}; diff --git a/+file/Group.m b/+file/Group.m index 669a61c81..abb6108ac 100644 --- a/+file/Group.m +++ b/+file/Group.m @@ -161,13 +161,13 @@ PropertyMap = [PropertyMap; Sub_Attribute_Map]; end PropertyMap(SubData.name) = SubData; - else - hoistedAttributes = getHoistedTypedDatasetAttributes(obj, SubData); - if ~isempty(hoistedAttributes) - attrNames = {hoistedAttributes.name}; + else % Typed dataset + includedAttributes = getIncludedTypedDatasetAttributes(obj, SubData); + if ~isempty(includedAttributes) + attrNames = {includedAttributes.name}; attrNames = strcat(SubData.name, '_', attrNames); PropertyMap = [PropertyMap; ... - containers.Map(attrNames, num2cell(hoistedAttributes))]; + containers.Map(attrNames, num2cell(includedAttributes))]; end if isempty(SubData.name) PropertyMap(lower(SubData.type)) = SubData; @@ -226,7 +226,7 @@ for iSubGroup = 1:length(descendantNames) descendantName = descendantNames{iSubGroup}; Descendant = DescendantMap(descendantName); - % hoist constrained sets to the current + % bubble constrained sets up to the current % subname. isPossiblyConstrained =... isa(Descendant, 'file.Group')... @@ -261,8 +261,18 @@ end end -function hoistedAttributes = getHoistedTypedDatasetAttributes(GroupObj, datasetObj) - hoistedAttributes = file.Attribute.empty; +function includedAttributes = getIncludedTypedDatasetAttributes(GroupObj, datasetObj) +% getIncludedTypedDatasetAttributes - Return attributes declared on a named +% included typed dataset instance. +% +% This is used for reuse by inclusion (`neurodata_type_inc` without +% `neurodata_type_def`), where an existing typed dataset is embedded as a +% named component of another type. Promotion decisions are resolved later, +% once namespace context is available, so we can distinguish newly added +% attributes from modifications of attributes already defined on the +% included dataset type. + + includedAttributes = file.Attribute.empty; if isempty(GroupObj.type) || isempty(datasetObj.name) || isempty(datasetObj.attributes) return; end @@ -270,7 +280,6 @@ for iAttr = 1:length(datasetObj.attributes) attribute = datasetObj.attributes(iAttr); attribute.dependent = datasetObj.name; - attribute.dependent_typed = true; - hoistedAttributes(end+1) = attribute; %#ok + includedAttributes(end+1) = attribute; %#ok end end diff --git a/+file/fillClass.m b/+file/fillClass.m index 138d91186..96ecfe16e 100644 --- a/+file/fillClass.m +++ b/+file/fillClass.m @@ -4,7 +4,6 @@ %% PROCESSING class = processed(1); - classprops = file.internal.filterClassPropsForCodegen(classprops, namespace); allProperties = keys(classprops); required = {}; @@ -13,6 +12,7 @@ defaults = {}; dependent = {}; hidden = {}; % special hidden properties for hard-coded workarounds + %separate into readonly, required, and optional properties for iGroup = 1:length(allProperties) propertyName = allProperties{iGroup}; diff --git a/+file/fillConstructor.m b/+file/fillConstructor.m index 020ec2caa..9a80eae25 100644 --- a/+file/fillConstructor.m +++ b/+file/fillConstructor.m @@ -57,7 +57,7 @@ if isempty(names) return; end - % if there's a root object that is a constrained set, let it be hoistable from dynamic arguments + % if there's a root object that is a constrained set, let it be reachable from dynamic arguments dynamicConstrained = false(size(names)); isAnonymousType = false(size(names)); isAttribute = false(size(names)); @@ -343,4 +343,4 @@ function assertValidRefType(referenceType) word (1,:) char end word(1) = upper(word(1)); -end \ No newline at end of file +end diff --git a/+file/fillExport.m b/+file/fillExport.m index e198415cc..5eb92aa1e 100644 --- a/+file/fillExport.m +++ b/+file/fillExport.m @@ -261,7 +261,7 @@ sprintf('obj.throwErrorIfRequiredDependencyMissing(''%s'', ''%s'', fullpath)', name, depPropname); end - if prop.dependent_typed + if prop.promoted_to_container preExportString = sprintf([ ... 'if isempty(obj.%1$s) && ~isempty(obj.%2$s) && isobject(obj.%2$s) && isprop(obj.%2$s, ''%3$s'') && ~isempty(obj.%2$s.%3$s)\n' ... ' obj.%1$s = obj.%2$s.%3$s;\n' ... diff --git a/+file/fillSetters.m b/+file/fillSetters.m index 379a2ad92..18e08d7aa 100644 --- a/+file/fillSetters.m +++ b/+file/fillSetters.m @@ -44,9 +44,9 @@ 'obj.warnIfAttributeDependencyMissing(''%s'', ''%s'')', ... propname, parentname); - syncTypedDatasetAttributeString = ''; - if prop.dependent_typed - syncTypedDatasetAttributeString = sprintf([ ... + syncPromotedDatasetAttributeString = ''; + if prop.promoted_to_container + syncPromotedDatasetAttributeString = sprintf([ ... 'if ~isempty(obj.%1$s) && isobject(obj.%1$s) && isprop(obj.%1$s, ''%2$s'')\n' ... ' obj.%1$s.%2$s = obj.%3$s;\n' ... 'end'], parentname, prop.name, propname); @@ -58,8 +58,8 @@ file.addSpaces(warnIfDependencyMissingString, 8), ... file.addSpaces('end', 4), ... 'end'}; - if ~isempty(syncTypedDatasetAttributeString) - postsetLines = [postsetLines(1:end-1), {file.addSpaces(syncTypedDatasetAttributeString, 4)}, postsetLines(end)]; + if ~isempty(syncPromotedDatasetAttributeString) + postsetLines = [postsetLines(1:end-1), {file.addSpaces(syncPromotedDatasetAttributeString, 4)}, postsetLines(end)]; end postsetFunctionStr = strjoin(postsetLines, newline); end diff --git a/+file/processClass.m b/+file/processClass.m index 0e94fb116..09a418d49 100644 --- a/+file/processClass.m +++ b/+file/processClass.m @@ -32,6 +32,7 @@ class = patchVectorData(class); end props = class.getProps(); + props = markPromotedAttributesForIncludedTypedDatasets(class, props, namespace); % Apply patches for special cases of schema/specification errors class = applySchemaVersionPatches(nodename, class, props, namespace); @@ -53,6 +54,81 @@ end end +function props = markPromotedAttributesForIncludedTypedDatasets(classObj, props, namespace) + if ~isa(classObj, 'file.Group') || isempty(classObj.datasets) + return; + end + + for iDataset = 1:length(classObj.datasets) + datasetObj = classObj.datasets(iDataset); + if isempty(datasetObj.type) || isempty(datasetObj.name) || isempty(datasetObj.attributes) + continue; + end + + datasetNamespace = namespace.getNamespace(datasetObj.type); + if isempty(datasetNamespace) + continue; + end + + schemaAttributeNames = getSchemaDefinedAttributeNames(datasetObj.type, datasetNamespace); + for iAttr = 1:length(datasetObj.attributes) + attribute = datasetObj.attributes(iAttr); + propertyName = [datasetObj.name '_' attribute.name]; + if ~isKey(props, propertyName) + continue; + end + + if any(strcmp(attribute.name, schemaAttributeNames)) + remove(props, propertyName); + else + promotedAttribute = props(propertyName); + promotedAttribute.promoted_to_container = true; + props(propertyName) = promotedAttribute; + end + end + end +end + +function attributeNames = getSchemaDefinedAttributeNames(typeName, namespace) + persistent schemaAttributeNameCache + + if isempty(schemaAttributeNameCache) + schemaAttributeNameCache = containers.Map('KeyType', 'char', 'ValueType', 'any'); + end + + cacheKey = strjoin({namespace.name, namespace.version, typeName}, '::'); + if isKey(schemaAttributeNameCache, cacheKey) + attributeNames = schemaAttributeNameCache(cacheKey); + return; + end + + typeSpec = namespace.getClass(typeName); + if isempty(typeSpec) + attributeNames = {}; + return; + end + + branch = [{typeSpec} namespace.getRootBranch(typeName)]; + spec.internal.resolveInheritedFields(typeSpec, branch(2:end)) + spec.internal.expandFieldsInheritedByInclusion(typeSpec) + + switch typeSpec('class_type') + case 'groups' + classObj = file.Group(typeSpec); + case 'datasets' + classObj = file.Dataset(typeSpec); + otherwise + attributeNames = {}; + return; + end + + typeProps = classObj.getProps(); + propNames = keys(typeProps); + isAttribute = cellfun(@(name) isa(typeProps(name), 'file.Attribute'), propNames); + attributeNames = propNames(isAttribute); + schemaAttributeNameCache(cacheKey) = attributeNames; +end + function class = patchVectorData(class) %% Unit Attribute % derived from schema 2.6.0 @@ -95,4 +171,4 @@ source('required') = false; class.attributes(end+1) = file.Attribute(source); -end \ No newline at end of file +end diff --git a/+io/parseDataset.m b/+io/parseDataset.m index 27ddfab17..5a6bc4c88 100644 --- a/+io/parseDataset.m +++ b/+io/parseDataset.m @@ -1,9 +1,9 @@ -function parsed = parseDataset(filename, info, fullpath, Blacklist, parentTypeName) +function parsed = parseDataset(filename, info, fullpath, Blacklist, containerTypeName) %typed and untyped being container maps containing type and untyped datasets % the maps store information regarding information and stored data % NOTE, dataset name is in path format so we need to parse that out. if nargin < 5 - parentTypeName = ''; + containerTypeName = ''; end name = info.Name; @@ -19,14 +19,14 @@ parsed = containers.Map; afields = keys(attrargs); if ~isempty(afields) - hoistedFields = afields; - if ~isempty(Type.typename) && ~isempty(parentTypeName) - hoistedFields = filterHoistedFieldsForParent(parentTypeName, name, afields); + promotedFields = afields; + if ~isempty(Type.typename) && ~isempty(containerTypeName) + promotedFields = filterPromotedFieldsForContainer(containerTypeName, name, afields); end - if ~isempty(hoistedFields) - anames = strcat(name, '_', hoistedFields); - parsed = [parsed; containers.Map(anames, attrargs.values(hoistedFields))]; + if ~isempty(promotedFields) + anames = strcat(name, '_', promotedFields); + parsed = [parsed; containers.Map(anames, attrargs.values(promotedFields))]; end end @@ -99,14 +99,14 @@ H5F.close(fid); end -function hoistedFields = filterHoistedFieldsForParent(parentTypeName, datasetName, availableFields) - metaClass = meta.class.fromName(parentTypeName); +function promotedFields = filterPromotedFieldsForContainer(containerTypeName, datasetName, availableFields) + metaClass = meta.class.fromName(containerTypeName); if isempty(metaClass) - hoistedFields = availableFields; + promotedFields = availableFields; return; end - parentPropertyNames = {metaClass.PropertyList.Name}; + containerPropertyNames = {metaClass.PropertyList.Name}; prefixedFieldNames = strcat(datasetName, '_', availableFields); - hoistedFields = availableFields(ismember(prefixedFieldNames, parentPropertyNames)); + promotedFields = availableFields(ismember(prefixedFieldNames, containerPropertyNames)); end diff --git a/+schemes/+internal/getRequiredPropsForClass.m b/+schemes/+internal/getRequiredPropsForClass.m index da9fc9ea5..48a2ac7e0 100644 --- a/+schemes/+internal/getRequiredPropsForClass.m +++ b/+schemes/+internal/getRequiredPropsForClass.m @@ -33,8 +33,6 @@ end classprops = file.internal.mergeProps(classprops, superClassProps); end - classprops = file.internal.filterClassPropsForCodegen(classprops, namespace); - % Resolve the required properties. For the final list of required properties, % we ignore both hidden and read-only properties. allPropertieNames = keys(classprops); diff --git a/+tests/+system/UnitTimesIOTest.m b/+tests/+system/UnitTimesIOTest.m index 62ec51d9e..786e7bbfb 100644 --- a/+tests/+system/UnitTimesIOTest.m +++ b/+tests/+system/UnitTimesIOTest.m @@ -40,7 +40,7 @@ function addContainer(~, file) , 'data', 1 ... ); - % set optional Units table dataset attributes via hoisted API + % set optional Units table dataset attributes via promoted container API file.units.spike_times_resolution = 3; Units = file.units; [Units.waveform_mean_sampling_rate ... From b5419e3267faadc6132681197a6c5a23c88fd1f8 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 30 Mar 2026 09:38:19 +0200 Subject: [PATCH 3/7] Restore file.fillConstructor Restore formatting changes unrelated to PR --- +file/fillConstructor.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/+file/fillConstructor.m b/+file/fillConstructor.m index 9a80eae25..020ec2caa 100644 --- a/+file/fillConstructor.m +++ b/+file/fillConstructor.m @@ -57,7 +57,7 @@ if isempty(names) return; end - % if there's a root object that is a constrained set, let it be reachable from dynamic arguments + % if there's a root object that is a constrained set, let it be hoistable from dynamic arguments dynamicConstrained = false(size(names)); isAnonymousType = false(size(names)); isAttribute = false(size(names)); @@ -343,4 +343,4 @@ function assertValidRefType(referenceType) word (1,:) char end word(1) = upper(word(1)); -end +end \ No newline at end of file From 9a353bdc9797d2fb2cc7b37de0b2ce3ef9f25dfa Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 30 Mar 2026 09:39:27 +0200 Subject: [PATCH 4/7] Update Group.m Restore comment wording --- +file/Group.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/+file/Group.m b/+file/Group.m index abb6108ac..dd9c54552 100644 --- a/+file/Group.m +++ b/+file/Group.m @@ -226,7 +226,7 @@ for iSubGroup = 1:length(descendantNames) descendantName = descendantNames{iSubGroup}; Descendant = DescendantMap(descendantName); - % bubble constrained sets up to the current + % hoist constrained sets up to the current % subname. isPossiblyConstrained =... isa(Descendant, 'file.Group')... From 4f0c1eac42c5d7ceb2dcb045e64bf515bde56a82 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 31 Mar 2026 11:53:55 +0200 Subject: [PATCH 5/7] Update parseDataset.m --- +io/parseDataset.m | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/+io/parseDataset.m b/+io/parseDataset.m index d43dec444..89c7d43d4 100644 --- a/+io/parseDataset.m +++ b/+io/parseDataset.m @@ -162,15 +162,3 @@ attributeValues = values(attributes, attributeNames); promotedAttributes = containers.Map(promotedAttributeNames, attributeValues); end - -function promotedFields = filterPromotedFieldsForContainer(containerTypeName, datasetName, availableFields) - metaClass = meta.class.fromName(containerTypeName); - if isempty(metaClass) - promotedFields = availableFields; - return; - end - - containerPropertyNames = {metaClass.PropertyList.Name}; - prefixedFieldNames = strcat(datasetName, '_', availableFields); - promotedFields = availableFields(ismember(prefixedFieldNames, containerPropertyNames)); -end From df40f88eb00569d3b2d41ec27f443bdf5a918cb0 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 31 Mar 2026 12:29:32 +0200 Subject: [PATCH 6/7] restore unrelated changes --- +file/Group.m | 2 +- +schemes/+internal/getRequiredPropsForClass.m | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/+file/Group.m b/+file/Group.m index dd9c54552..34463c06e 100644 --- a/+file/Group.m +++ b/+file/Group.m @@ -226,7 +226,7 @@ for iSubGroup = 1:length(descendantNames) descendantName = descendantNames{iSubGroup}; Descendant = DescendantMap(descendantName); - % hoist constrained sets up to the current + % hoist constrained sets to the current % subname. isPossiblyConstrained =... isa(Descendant, 'file.Group')... diff --git a/+schemes/+internal/getRequiredPropsForClass.m b/+schemes/+internal/getRequiredPropsForClass.m index 48a2ac7e0..5d6111834 100644 --- a/+schemes/+internal/getRequiredPropsForClass.m +++ b/+schemes/+internal/getRequiredPropsForClass.m @@ -33,6 +33,7 @@ end classprops = file.internal.mergeProps(classprops, superClassProps); end + % Resolve the required properties. For the final list of required properties, % we ignore both hidden and read-only properties. allPropertieNames = keys(classprops); From ab5e3f52514d3c2b67ec5043cc3dc9a82acc93de Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 31 Mar 2026 12:39:40 +0200 Subject: [PATCH 7/7] Fix legacy compatibility resolution property of spike_times VectorData object was previously cleared when spike_times was added to the units object --- +file/fillSetters.m | 6 +++++- +tests/+system/UnitTimesIOTest.m | 17 +++++++++++++++++ +types/+core/Units.m | 24 ++++++++++++++++++++---- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/+file/fillSetters.m b/+file/fillSetters.m index 18e08d7aa..d209f3d4b 100644 --- a/+file/fillSetters.m +++ b/+file/fillSetters.m @@ -48,7 +48,11 @@ if prop.promoted_to_container syncPromotedDatasetAttributeString = sprintf([ ... 'if ~isempty(obj.%1$s) && isobject(obj.%1$s) && isprop(obj.%1$s, ''%2$s'')\n' ... - ' obj.%1$s.%2$s = obj.%3$s;\n' ... + ' if ~isempty(obj.%3$s)\n' ... + ' obj.%1$s.%2$s = obj.%3$s;\n' ... + ' elseif ~isempty(obj.%1$s.%2$s)\n' ... + ' obj.%3$s = obj.%1$s.%2$s;\n' ... + ' end\n' ... 'end'], parentname, prop.name, propname); end diff --git a/+tests/+system/UnitTimesIOTest.m b/+tests/+system/UnitTimesIOTest.m index 786e7bbfb..276d03b6a 100644 --- a/+tests/+system/UnitTimesIOTest.m +++ b/+tests/+system/UnitTimesIOTest.m @@ -53,4 +53,21 @@ function addContainer(~, file) c = file.units; end end + + methods (Test) + function testLegacyNestedSpikeTimesResolutionIsPreserved(testCase) + spikeTimes = types.hdmf_common.VectorData( ... + 'data', 11, ... + 'description', 'the spike times for each unit in seconds'); + spikeTimes.resolution = 1/20000; + + units = types.core.Units( ... + 'colnames', {'spike_times'}, ... + 'description', 'data on spiking units', ... + 'spike_times', spikeTimes); + + testCase.verifyEqual(units.spike_times.resolution, 1/20000); + testCase.verifyEqual(units.spike_times_resolution, 1/20000); + end + end end diff --git a/+types/+core/Units.m b/+types/+core/Units.m index e4bd64dc3..3c1c6a240 100644 --- a/+types/+core/Units.m +++ b/+types/+core/Units.m @@ -170,7 +170,11 @@ function postset_spike_times_resolution(obj) obj.warnIfAttributeDependencyMissing('spike_times_resolution', 'spike_times') end if ~isempty(obj.spike_times) && isobject(obj.spike_times) && isprop(obj.spike_times, 'resolution') - obj.spike_times.resolution = obj.spike_times_resolution; + if ~isempty(obj.spike_times_resolution) + obj.spike_times.resolution = obj.spike_times_resolution; + elseif ~isempty(obj.spike_times.resolution) + obj.spike_times_resolution = obj.spike_times.resolution; + end end end function set.waveform_mean(obj, val) @@ -185,7 +189,11 @@ function postset_waveform_mean_sampling_rate(obj) obj.warnIfAttributeDependencyMissing('waveform_mean_sampling_rate', 'waveform_mean') end if ~isempty(obj.waveform_mean) && isobject(obj.waveform_mean) && isprop(obj.waveform_mean, 'sampling_rate') - obj.waveform_mean.sampling_rate = obj.waveform_mean_sampling_rate; + if ~isempty(obj.waveform_mean_sampling_rate) + obj.waveform_mean.sampling_rate = obj.waveform_mean_sampling_rate; + elseif ~isempty(obj.waveform_mean.sampling_rate) + obj.waveform_mean_sampling_rate = obj.waveform_mean.sampling_rate; + end end end function set.waveform_sd(obj, val) @@ -200,7 +208,11 @@ function postset_waveform_sd_sampling_rate(obj) obj.warnIfAttributeDependencyMissing('waveform_sd_sampling_rate', 'waveform_sd') end if ~isempty(obj.waveform_sd) && isobject(obj.waveform_sd) && isprop(obj.waveform_sd, 'sampling_rate') - obj.waveform_sd.sampling_rate = obj.waveform_sd_sampling_rate; + if ~isempty(obj.waveform_sd_sampling_rate) + obj.waveform_sd.sampling_rate = obj.waveform_sd_sampling_rate; + elseif ~isempty(obj.waveform_sd.sampling_rate) + obj.waveform_sd_sampling_rate = obj.waveform_sd.sampling_rate; + end end end function set.waveforms(obj, val) @@ -221,7 +233,11 @@ function postset_waveforms_sampling_rate(obj) obj.warnIfAttributeDependencyMissing('waveforms_sampling_rate', 'waveforms') end if ~isempty(obj.waveforms) && isobject(obj.waveforms) && isprop(obj.waveforms, 'sampling_rate') - obj.waveforms.sampling_rate = obj.waveforms_sampling_rate; + if ~isempty(obj.waveforms_sampling_rate) + obj.waveforms.sampling_rate = obj.waveforms_sampling_rate; + elseif ~isempty(obj.waveforms.sampling_rate) + obj.waveforms_sampling_rate = obj.waveforms.sampling_rate; + end end end %% VALIDATORS