From 4a305d0fc687575ee7512f2a0d76ec6e4258a478 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 7 Nov 2024 20:44:31 +0100 Subject: [PATCH 01/32] Refactor: Add arguments block to main MatNWB api functions - Move some functions io.spec namespace - Add validation functions in matnwb.common namespace --- +file/writeNamespace.m | 2 +- .../+internal/readEmbeddedSpecLocation.m | 18 ++ +io/+spec/getEmbeddedSpecLocation.m | 16 ++ +io/+spec/readEmbeddedSpecifications.m | 60 ++++++ +io/+spec/writeEmbeddedSpecifications.m | 45 +++++ +matnwb/+common/findLatestSchemaVersion.m | 20 ++ +matnwb/+common/mustBeNwbFile.m | 7 + +matnwb/+common/mustBeValidSchemaVersion.m | 30 +++ +spec/generate.m | 6 +- +tests/+system/NWBFileIOTest.m | 25 ++- NwbFile.m | 99 +++------- generateCore.m | 51 +++-- generateExtension.m | 71 ++++--- nwbClearGenerated.m | 7 +- nwbExport.m | 41 ++--- nwbRead.m | 174 ++++++------------ 16 files changed, 390 insertions(+), 282 deletions(-) create mode 100644 +io/+spec/+internal/readEmbeddedSpecLocation.m create mode 100644 +io/+spec/getEmbeddedSpecLocation.m create mode 100644 +io/+spec/readEmbeddedSpecifications.m create mode 100644 +io/+spec/writeEmbeddedSpecifications.m create mode 100644 +matnwb/+common/findLatestSchemaVersion.m create mode 100644 +matnwb/+common/mustBeNwbFile.m create mode 100644 +matnwb/+common/mustBeValidSchemaVersion.m diff --git a/+file/writeNamespace.m b/+file/writeNamespace.m index 95a77b65a..10e6ef537 100644 --- a/+file/writeNamespace.m +++ b/+file/writeNamespace.m @@ -4,7 +4,7 @@ function writeNamespace(namespaceName, saveDir) classFileDir = fullfile(saveDir, '+types', ['+' misc.str2validName(Namespace.name)]); -if 7 ~= exist(classFileDir, 'dir') +if ~isfolder(classFileDir) mkdir(classFileDir); end diff --git a/+io/+spec/+internal/readEmbeddedSpecLocation.m b/+io/+spec/+internal/readEmbeddedSpecLocation.m new file mode 100644 index 000000000..15a28908a --- /dev/null +++ b/+io/+spec/+internal/readEmbeddedSpecLocation.m @@ -0,0 +1,18 @@ +function specLocation = readEmbeddedSpecLocation(fid, specLocAttributeName) + arguments + fid (1,1) H5ML.id + specLocAttributeName (1,1) string = '.specloc' + end + + specLocation = ''; + try % Check .specloc + attributeId = H5A.open(fid, specLocAttributeName); + attributeCleanup = onCleanup(@(id) H5A.close(attributeId)); + referenceRawData = H5A.read(attributeId); + specLocation = H5R.get_name(attributeId, 'H5R_OBJECT', referenceRawData); + catch ME + if ~strcmp(ME.identifier, 'MATLAB:imagesci:hdf5lib:libraryError') + rethrow(ME); + end % don't error if the attribute doesn't exist. + end +end \ No newline at end of file diff --git a/+io/+spec/getEmbeddedSpecLocation.m b/+io/+spec/getEmbeddedSpecLocation.m new file mode 100644 index 000000000..32215d9c0 --- /dev/null +++ b/+io/+spec/getEmbeddedSpecLocation.m @@ -0,0 +1,16 @@ +function specLocation = getEmbeddedSpecLocation(filename, options) +% getEmbeddedSpecLocation - Get location of embedded specs in NWB file +% +% Note: Returns an empty string if the spec location does not exist +% +% See also io.spec.internal.readEmbeddedSpecLocation + + arguments + filename (1,1) string {matnwb.common.mustBeNwbFile} + options.SpecLocAttributeName (1,1) string = '.specloc' + end + + fid = H5F.open(filename); + fileCleanup = onCleanup(@(id) H5F.close(fid) ); + specLocation = io.spec.internal.readEmbeddedSpecLocation(fid, options.SpecLocAttributeName); +end diff --git a/+io/+spec/readEmbeddedSpecifications.m b/+io/+spec/readEmbeddedSpecifications.m new file mode 100644 index 000000000..919e2fc9e --- /dev/null +++ b/+io/+spec/readEmbeddedSpecifications.m @@ -0,0 +1,60 @@ +function specs = readEmbeddedSpecifications(filename, specLocation) +% readEmbeddedSpecifications - Read embedded specs from an NWB file +% +% specs = io.spec.readEmbeddedSpecifications(filename, specLocation) read +% embedded specs from the specLocation in an NWB file +% +% Inputs: +% filename (string) : Absolute path of an nwb file +% specLocation (string) : h5 path for the location of specs inside the NWB file +% +% Outputs +% specs cell: A cell array of structs with one element for each embedded +% specification. Each struct has two fields: +% +% - namespaceName (char) : Name of the namespace for a specification +% - namespaceText (char) : The namespace declaration for a specification +% - schemaMap (containers.Map): A set of schema specifications for the namespace + + arguments + filename (1,1) string {matnwb.common.mustBeNwbFile} + specLocation (1,1) string + end + + specInfo = h5info(filename, specLocation); + specs = deal( cell(size(specInfo.Groups)) ); + + fid = H5F.open(filename); + fileCleanup = onCleanup(@(id) H5F.close(fid) ); + + for iGroup = 1:length(specInfo.Groups) + location = specInfo.Groups(iGroup).Groups(1); + + namespaceName = split(specInfo.Groups(iGroup).Name, '/'); + namespaceName = namespaceName{end}; + + filenames = {location.Datasets.Name}; + if ~any(strcmp('namespace', filenames)) + warning('NWB:Read:GenerateSpec:CacheInvalid',... + 'Couldn''t find a `namespace` in namespace `%s`. Skipping cache generation.',... + namespaceName); + return; + end + sourceNames = {location.Datasets.Name}; + fileLocation = strcat(location.Name, '/', sourceNames); + schemaMap = containers.Map; + for iFileLocation = 1:length(fileLocation) + did = H5D.open(fid, fileLocation{iFileLocation}); + if strcmp('namespace', sourceNames{iFileLocation}) + namespaceText = H5D.read(did); + else + schemaMap(sourceNames{iFileLocation}) = H5D.read(did); + end + H5D.close(did); + end + + specs{iGroup}.namespaceName = namespaceName; + specs{iGroup}.namespaceText = namespaceText; + specs{iGroup}.schemaMap = schemaMap; + end +end diff --git a/+io/+spec/writeEmbeddedSpecifications.m b/+io/+spec/writeEmbeddedSpecifications.m new file mode 100644 index 000000000..23c2e269f --- /dev/null +++ b/+io/+spec/writeEmbeddedSpecifications.m @@ -0,0 +1,45 @@ +function writeEmbeddedSpecifications(fid, jsonSpecs) + specLocation = io.spec.internal.readEmbeddedSpecLocation(fid); + + if isempty(specLocation) + specLocation = '/specifications'; + io.writeGroup(fid, specLocation); + specView = types.untyped.ObjectView(specLocation); + io.writeAttribute(fid, '/.specloc', specView); + end + + for iJson = 1:length(jsonSpecs) + JsonDatum = jsonSpecs(iJson); + schemaNamespaceLocation = strjoin({specLocation, JsonDatum.name}, '/'); + namespaceExists = io.writeGroup(fid, schemaNamespaceLocation); + if namespaceExists + namespaceGroupId = H5G.open(fid, schemaNamespaceLocation); + names = getVersionNames(namespaceGroupId); + H5G.close(namespaceGroupId); + for iNames = 1:length(names) + H5L.delete(fid, [schemaNamespaceLocation '/' names{iNames}],... + 'H5P_DEFAULT'); + end + end + schemaLocation =... + strjoin({schemaNamespaceLocation, JsonDatum.version}, '/'); + io.writeGroup(fid, schemaLocation); + Json = JsonDatum.json; + schemeNames = keys(Json); + for iScheme = 1:length(schemeNames) + name = schemeNames{iScheme}; + path = [schemaLocation '/' name]; + io.writeDataset(fid, path, Json(name)); + end + end +end + +function versionNames = getVersionNames(namespaceGroupId) + [~, ~, versionNames] = H5L.iterate(namespaceGroupId,... + 'H5_INDEX_NAME', 'H5_ITER_NATIVE',... + 0, @removeGroups, {}); + function [status, versionNames] = removeGroups(~, name, versionNames) + versionNames{end+1} = name; + status = 0; + end +end diff --git a/+matnwb/+common/findLatestSchemaVersion.m b/+matnwb/+common/findLatestSchemaVersion.m new file mode 100644 index 000000000..5ab078e2a --- /dev/null +++ b/+matnwb/+common/findLatestSchemaVersion.m @@ -0,0 +1,20 @@ +function latestVersion = findLatestSchemaVersion() +% findLatestSchemaVersion - Find latest available schema version. + + schemaListing = dir(fullfile(misc.getMatnwbDir(), 'nwb-schema')); + schemaVersionNumbers = setdiff({schemaListing.name}, {'.', '..'}); + + % Split each version number into major, minor, and patch components + versionComponents = cellfun(@(v) sscanf(v, '%d.%d.%d'), ... + schemaVersionNumbers, 'UniformOutput', false); + + % Convert the components into an array for easy comparison + versionMatrix = cat(2, versionComponents{:})'; + + % Find the row with the highest version number, weighting major + % and minor with factors of 6 and 3 respectively + [~, latestIndex] = max(versionMatrix * [1e6; 1e3; 1]); % Weight major, minor, patch + + % Return the latest version + latestVersion = schemaVersionNumbers{latestIndex}; +end \ No newline at end of file diff --git a/+matnwb/+common/mustBeNwbFile.m b/+matnwb/+common/mustBeNwbFile.m new file mode 100644 index 000000000..50b21777c --- /dev/null +++ b/+matnwb/+common/mustBeNwbFile.m @@ -0,0 +1,7 @@ +function mustBeNwbFile(filePath) +% mustBeNwbFile - Check that file path points to existing file with .nwb extension + arguments + filePath (1,1) string {mustBeFile} + end + assert(endsWith(filePath, ".nwb", "IgnoreCase", true)) +end \ No newline at end of file diff --git a/+matnwb/+common/mustBeValidSchemaVersion.m b/+matnwb/+common/mustBeValidSchemaVersion.m new file mode 100644 index 000000000..a44c9d605 --- /dev/null +++ b/+matnwb/+common/mustBeValidSchemaVersion.m @@ -0,0 +1,30 @@ +function mustBeValidSchemaVersion(versionNumber) +% mustBeValidSchemaVersion - Validate version number against available schemas + arguments + versionNumber (1,1) string + end + + persistent schemaVersionNumbers + + if versionNumber == "latest" + return % Should be resolved downstream. + end + + versionPattern = "^\d+\.\d+\.\d+$"; % i.e 2.0.0 + if isempty(regexp(versionNumber, versionPattern, 'once')) + error('NWB:VersionValidator:InvalidVersionNumber', ... + "Version number should formatted as ..") + end + + % Validate supported schema version + if isempty(schemaVersionNumbers) + schemaListing = dir(fullfile(misc.getMatnwbDir(), 'nwb-schema')); + schemaVersionNumbers = setdiff({schemaListing.name}, {'.', '..'}); + end + + if ~any(strcmp(versionNumber, schemaVersionNumbers)) + error('NWB:VersionValidator:UnsupportedSchemaVersion', ... + "The provided version number ('%s') is not supported by this version of MatNWB", ... + versionNumber) + end +end diff --git a/+spec/generate.m b/+spec/generate.m index b452f38d2..f6dfcffbd 100644 --- a/+spec/generate.m +++ b/+spec/generate.m @@ -9,15 +9,13 @@ for iInfo = 1:length(Namespaces) Namespaces(iInfo).namespace = namespace; - if ischar(schemaSource) + if ischar(schemaSource) || isstring(schemaSource) schema = containers.Map; Namespace = Namespaces(iInfo); for iFilenames = 1:length(Namespace.filenames) filenameStub = Namespace.filenames{iFilenames}; filename = [filenameStub '.yaml']; - fid = fopen(fullfile(schemaSource, filename)); - schema(filenameStub) = fread(fid, '*char') .'; - fclose(fid); + schema(filenameStub) = fileread(fullfile(schemaSource, filename)); end schema = spec.getSourceInfo(schema); else % map of schemas with their locations diff --git a/+tests/+system/NWBFileIOTest.m b/+tests/+system/NWBFileIOTest.m index 2473e34d0..f9747acde 100644 --- a/+tests/+system/NWBFileIOTest.m +++ b/+tests/+system/NWBFileIOTest.m @@ -61,7 +61,9 @@ function readFileWithoutSpecLoc(testCase) testCase.deleteAttributeFromFile(fileName, '/', '.specloc') - nwbRead(fileName); + % When specloc is missing, the specifications are not added to + % the blacklist, so it will get passed as an input to NwbFile. + testCase.verifyError(@(fn) nwbRead(fileName), 'MATLAB:TooManyInputs'); end function readFileWithUnsupportedVersion(testCase) @@ -74,7 +76,26 @@ function readFileWithUnsupportedVersion(testCase) io.writeAttribute(file_id, '/nwb_version', '1.0.0') H5F.close(file_id); - nwbRead(fileName); + testCase.verifyWarning(@(fn) nwbRead(fileName), 'NWB:Read:UnsupportedSchema') + end + + function readFileWithUnsupportedVersionAndNoSpecloc(testCase) + import matlab.unittest.fixtures.SuppressedWarningsFixture + testCase.applyFixture(SuppressedWarningsFixture('NWB:Read:UnsupportedSchema')) + + fileName = ['MatNWB.' testCase.className() '.testReadFileWithUnsupportedVersionAndNoSpecloc.nwb']; + nwbExport(testCase.file, fileName) + + testCase.deleteAttributeFromFile(fileName, '/', '.specloc') + testCase.deleteAttributeFromFile(fileName, '/', 'nwb_version') + + file_id = H5F.open(fileName, 'H5F_ACC_RDWR', 'H5P_DEFAULT'); + io.writeAttribute(file_id, '/nwb_version', '1.0.0') + H5F.close(file_id); + + % When specloc is missing, the specifications are not added to + % the blacklist, so it will get passed as an input to NwbFile. + testCase.verifyError(@(fn) nwbRead(fileName), 'MATLAB:TooManyInputs'); end end diff --git a/NwbFile.m b/NwbFile.m index cc88b1ddb..f89ac708e 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -12,17 +12,29 @@ % See also NWBREAD, GENERATECORE, GENERATEEXTENSION methods - function obj = NwbFile(varargin) - obj = obj@types.core.NWBFile(varargin{:}); - if strcmp(class(obj), 'NwbFile') - cellStringArguments = convertContainedStringsToChars(varargin(1:2:end)); + function obj = NwbFile(propValues) + arguments + propValues.?types.core.NWBFile + propValues.nwb_version + end + nameValuePairs = namedargs2cell(propValues); + obj = obj@types.core.NWBFile(nameValuePairs{:}); + if strcmp(class(obj), 'NwbFile') %#ok + cellStringArguments = convertContainedStringsToChars(nameValuePairs(1:2:end)); types.util.checkUnset(obj, unique(cellStringArguments)); end end - function export(obj, filename) - %add to file create date - + function export(obj, filename, mode) + % export - Export NWB file object + + arguments + obj (1,1) NwbFile + filename (1,1) string + mode (1,1) string {mustBeMember(mode, ["edit", "overwrite"])} = "edit" + end + + % add to file create date if isa(obj.file_create_date, 'types.untyped.DataStub') obj.file_create_date = obj.file_create_date.load(); end @@ -43,26 +55,22 @@ function export(obj, filename) obj.timestamps_reference_time = obj.session_start_time; end - try - output_file_id = H5F.create(filename); - isEditingFile = false; - catch ME % if file exists, open and edit - if verLessThan('matlab', '9.9') % < 2020b - isEditingFile = strcmp(ME.identifier, 'MATLAB:imagesci:hdf5lib:libraryError')... - && contains(ME.message, '''File exists'''); - else - isEditingFile = strcmp(ME.identifier, 'MATLAB:imagesci:hdf5io:resourceAlreadyExists'); - end + isEditingFile = false; - if isEditingFile + if isfile(filename) + if mode == "edit" output_file_id = H5F.open(filename, 'H5F_ACC_RDWR', 'H5P_DEFAULT'); - else - rethrow(ME); + isEditingFile = true; + elseif mode == "overwrite" + output_file_id = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); end + else + output_file_id = H5F.create(filename); end try - obj.embedSpecifications(output_file_id); + jsonSpecs = schemes.exportJson(); + io.spec.writeEmbeddedSpecifications(output_file_id, jsonSpecs); refs = export@types.core.NWBFile(obj, output_file_id, '/', {}); obj.resolveReferences(output_file_id, refs); H5F.close(output_file_id); @@ -130,55 +138,6 @@ function resolveReferences(obj, fid, references) references(resolved) = []; end end - - function embedSpecifications(~, fid) - try - attrId = H5A.open(fid, '/.specloc'); - specLocation = H5R.get_name(fid, 'H5R_OBJECT', H5A.read(attrId)); - H5A.close(attrId); - catch - specLocation = '/specifications'; - io.writeGroup(fid, specLocation); - specView = types.untyped.ObjectView(specLocation); - io.writeAttribute(fid, '/.specloc', specView); - end - - JsonData = schemes.exportJson(); - for iJson = 1:length(JsonData) - JsonDatum = JsonData(iJson); - schemaNamespaceLocation = strjoin({specLocation, JsonDatum.name}, '/'); - namespaceExists = io.writeGroup(fid, schemaNamespaceLocation); - if namespaceExists - namespaceGroupId = H5G.open(fid, schemaNamespaceLocation); - names = getVersionNames(namespaceGroupId); - H5G.close(namespaceGroupId); - for iNames = 1:length(names) - H5L.delete(fid, [schemaNamespaceLocation '/' names{iNames}],... - 'H5P_DEFAULT'); - end - end - schemaLocation =... - strjoin({schemaNamespaceLocation, JsonDatum.version}, '/'); - io.writeGroup(fid, schemaLocation); - Json = JsonDatum.json; - schemeNames = keys(Json); - for iScheme = 1:length(schemeNames) - name = schemeNames{iScheme}; - path = [schemaLocation '/' name]; - io.writeDataset(fid, path, Json(name)); - end - end - end - end -end - -function versionNames = getVersionNames(namespaceGroupId) - [~, ~, versionNames] = H5L.iterate(namespaceGroupId,... - 'H5_INDEX_NAME', 'H5_ITER_NATIVE',... - 0, @removeGroups, {}); - function [status, versionNames] = removeGroups(~, name, versionNames) - versionNames{end+1} = name; - status = 0; end end diff --git a/generateCore.m b/generateCore.m index 65ed9a278..03309e0f9 100644 --- a/generateCore.m +++ b/generateCore.m @@ -1,4 +1,4 @@ -function generateCore(varargin) +function generateCore(version, options) % GENERATECORE Generate Matlab classes from NWB core schema files % GENERATECORE() Generate classes (Matlab m-files) from the % NWB core namespace file. By default, generates off of the most recent nwb-schema @@ -22,34 +22,31 @@ function generateCore(varargin) % generateCore('2.2.3'); % % See also GENERATEEXTENSION - - latestVersion = '2.7.0'; - - if nargin == 0 || strcmp(varargin{1}, 'savedir') - version = latestVersion; - else - version = varargin{1}; - validateattributes(version, {'char', 'string'}, {'scalartext'}, 'generateCore', 'version', 1); - version = char(version); - varargin = varargin(2:end); + + arguments + version (1,1) string {matnwb.common.mustBeValidSchemaVersion} = "latest" + options.savedir (1,1) string = missing end - - if strcmp(version, 'latest') - version = latestVersion; + + if version == "latest" + version = matnwb.common.findLatestSchemaVersion(); end - - schemaPath = fullfile(misc.getMatnwbDir(), 'nwb-schema', version); - corePath = fullfile(schemaPath, 'core', 'nwb.namespace.yaml'); - commonPath = fullfile(schemaPath,... - 'hdmf-common-schema', ... - 'common',... - 'namespace.yaml'); - assert(2 == exist(corePath, 'file'),... - 'NWB:GenerateCore:MissingCoreSchema',... - 'Cannot find suitable core namespace for schema version `%s`',... + + schemaPath = fullfile(misc.getMatnwbDir(), "nwb-schema", version); + corePath = fullfile(schemaPath, "core", "nwb.namespace.yaml"); + commonPath = fullfile(schemaPath, ... + "hdmf-common-schema", ... + "common", ... + "namespace.yaml"); + assert(isfile(corePath), ... + 'NWB:GenerateCore:MissingCoreSchema', ... + 'Cannot find suitable core namespace for schema version `%s`', ... version); - if 2 == exist(commonPath, 'file') - generateExtension(commonPath, varargin{:}); + + namespaceFiles = corePath; + if isfile(commonPath) + % Important: generate common before core if common is available + namespaceFiles = [commonPath, namespaceFiles]; end - generateExtension(corePath, varargin{:}); + generateExtension(namespaceFiles{:}, 'savedir', options.savedir); end diff --git a/generateExtension.m b/generateExtension.m index 67085df3f..741264f69 100644 --- a/generateExtension.m +++ b/generateExtension.m @@ -1,4 +1,4 @@ -function generateExtension(varargin) +function generateExtension(namespaceFilePath, options) % GENERATEEXTENSION Generate Matlab classes from NWB extension schema file % GENERATEEXTENSION(extension_path...) Generate classes % (Matlab m-files) from one or more NWB schema extension namespace @@ -17,46 +17,41 @@ function generateExtension(varargin) % % See also GENERATECORE - for iOption = 1:length(varargin) - option = varargin{iOption}; - validateattributes(option, {'char', 'string'}, {'scalartext'} ... - , 'generateExtension', 'extension name', iOption); - if isstring(option) - varargin{iOption} = char(option); - end + arguments (Repeating) + namespaceFilePath (1,1) string {mustBeYamlFile} end - - saveDirMask = strcmp(varargin, 'savedir'); - if any(saveDirMask) - assert(~saveDirMask(end),... - 'NWB:GenerateExtenion:InvalidParameter',... - 'savedir must be paired with the desired save directory.'); - saveDir = varargin{find(saveDirMask, 1, 'last') + 1}; - saveDirParametersMask = saveDirMask | circshift(saveDirMask, 1); - sourceList = varargin(~saveDirParametersMask); - else - saveDir = misc.getMatnwbDir(); - sourceList = varargin; + arguments + options.savedir (1,1) string = misc.getMatnwbDir() end - - for iNamespaceFiles = 1:length(sourceList) - source = sourceList{iNamespaceFiles}; - validateattributes(source, {'char', 'string'}, {'scalartext'}); - - [localpath, ~, ~] = fileparts(source); - assert(2 == exist(source, 'file'),... - 'NWB:GenerateExtension:FileNotFound', 'Path to file `%s` could not be found.', source); - fid = fopen(source); - namespaceText = fread(fid, '*char') .'; - fclose(fid); - - Namespaces = spec.generate(namespaceText, localpath); + + if isempty(namespaceFilePath) + error('NWB:GenerateExtension:NamespaceMissing', ... + 'Please provide the file path to at least one namespace specification file.') + end + + for iNamespaceFiles = 1:length(namespaceFilePath) + + source = namespaceFilePath{iNamespaceFiles}; + namespaceText = fileread(source); + + [namespaceRootFolder, ~, ~] = fileparts(source); + parsedNamespaceList = spec.generate(namespaceText, namespaceRootFolder); - for iNamespace = 1:length(Namespaces) - Namespace = Namespaces(iNamespace); - spec.saveCache(Namespace, saveDir); - file.writeNamespace(Namespace.name, saveDir); - rehash(); + for iNamespace = 1:length(parsedNamespaceList) + parsedNamespace = parsedNamespaceList(iNamespace); + spec.saveCache(parsedNamespace, options.savedir); + file.writeNamespace(parsedNamespace.name, options.savedir); end end + rehash() +end + +function mustBeYamlFile(filePath) + arguments + filePath (1,1) string {mustBeFile} + end + + assert(endsWith(filePath, [".yaml", ".yml"], "IgnoreCase", true), ... + 'NWB:GenerateExtension:MustBeYaml', ... + 'Expected file to point to a yaml file', filePath) end diff --git a/nwbClearGenerated.m b/nwbClearGenerated.m index 0294c30b3..43a4bae12 100644 --- a/nwbClearGenerated.m +++ b/nwbClearGenerated.m @@ -1,4 +1,4 @@ -function nwbClearGenerated() +function clearedNamespaceNames = nwbClearGenerated() %% NWBCLEARGENERATED clears generated class files. nwbDir = misc.getMatnwbDir(); typesPath = fullfile(nwbDir, '+types'); @@ -8,4 +8,9 @@ function nwbClearGenerated() for i=1:length(generatedPaths) rmdir(generatedPaths{i}, 's'); end + if nargout == 1 % Return names of cleared namespaces + [~, clearedNamespaceNames] = fileparts(generatedPaths); + clearedNamespaceNames = strrep(clearedNamespaceNames, '+', ''); + clearedNamespaceNames = string(clearedNamespaceNames); + end end \ No newline at end of file diff --git a/nwbExport.m b/nwbExport.m index 204fc040e..8f5a451f2 100644 --- a/nwbExport.m +++ b/nwbExport.m @@ -1,6 +1,6 @@ -function nwbExport(nwb, filenames) +function nwbExport(nwbFileObjects, filePaths, mode) %NWBEXPORT Writes an NWB file. - % nwbRead(nwb,filename) Writes the nwb object to a file at filename. + % nwbRead(nwb, filename) Writes the nwb object to a file at filename. % % Example: % % Generate Matlab code for the NWB objects from the core schema. @@ -14,31 +14,20 @@ function nwbExport(nwb, filenames) % nwbExport(nwb, 'empty.nwb'); % % See also GENERATECORE, GENERATEEXTENSION, NWBFILE, NWBREAD - validateattributes(nwb, {'NwbFile'}, {'nonempty'}, 'nwbExport', 'nwb', 1); - validateattributes(filenames, {'cell', 'string', 'char'}, {'nonempty'}, 'nwbExport', 'filenames', 2); - if isstring(filenames) - filenames = convertStringsToChars(filenames); + + arguments + nwbFileObjects (1,:) NwbFile {mustBeNonempty} + filePaths (1,:) string {mustBeNonzeroLengthText} + mode (1,1) string {mustBeMember(mode, ["edit", "overwrite"])} = "edit" end - if iscell(filenames) - for iName = 1:length(filenames) - name = filenames{iName}; - validateattributes(name, {'string', 'char'}, {'scalartext', 'nonempty'} ... - , 'nwbExport', 'filenames', 2); - filenames{iName} = char(name); - end + + if length(nwbFileObjects) ~= length(filePaths) + error('NWB:Export:FilepathLengthMismatch', ... + 'Lists of NWB objects to export and list of file paths must be the same length.') end - if ~isscalar(nwb) - assert(~ischar(filenames) && length(filenames) == length(nwb), ... - 'NwbFile and filename array dimensions must match.'); - end - - for iFiles = 1:length(nwb) - if iscellstr(filenames) - filename = filenames{iFiles}; - else - filename = filenames; - end - - nwb(iFiles).export(filename); + + for iFiles = 1:length(nwbFileObjects) + filePath = char(filePaths(iFiles)); + nwbFileObjects(iFiles).export(filePath, mode); end end diff --git a/nwbRead.m b/nwbRead.m index fbc85846a..70ec5c6af 100644 --- a/nwbRead.m +++ b/nwbRead.m @@ -1,4 +1,4 @@ -function nwb = nwbRead(filename, varargin) +function nwb = nwbRead(filename, flags, options) %NWBREAD Reads an NWB file. % nwb = NWBREAD(filename) Reads the nwb file at filename and returns an % NWBFile object representing its contents. @@ -17,151 +17,98 @@ % % See also GENERATECORE, GENERATEEXTENSION, NWBFILE, NWBEXPORT - for iOption = 1:length(varargin) - option = varargin{iOption}; - if isstring(option) - option = char(option); - end - assert(ischar(option), 'NWB:Read:InvalidParameter' ... - , 'Invalid optional parameter in argument position %u', 1 + iOption); - varargin{iOption} = option; + arguments + filename (1,1) string {matnwb.common.mustBeNwbFile} end - - ignoreCache = any(strcmpi(varargin, 'ignorecache')); - - saveDirMask = strcmpi(varargin, 'savedir'); - assert(isempty(saveDirMask) || ~saveDirMask(end), 'NWB:NWBRead:InvalidSaveDir',... - '`savedir` is a key value pair requiring a directory string as a value.'); - if any(saveDirMask) - saveDir = varargin{find(saveDirMask, 1, 'last') + 1}; - else - saveDir = misc.getMatnwbDir(); + arguments (Repeating) + flags (1,1) string {mustBeMember(flags, "ignorecache")} end - - Blacklist = struct(... - 'attributes', {{'.specloc', 'object_id'}},... - 'groups', {{}}); - validateattributes(filename, {'char', 'string'}, {'scalartext', 'nonempty'} ... - , 'nwbRead', 'filename', 1); - - filename = char(filename); - specLocation = getEmbeddedSpec(filename); - schemaVersion = util.getSchemaVersion(filename); - if ~isempty(specLocation) - Blacklist.groups{end+1} = specLocation; + arguments + options.savedir (1,1) string = misc.getMatnwbDir(); % {mustBeFolder} ? end - % validate supported schema version - Schemas = dir(fullfile(misc.getMatnwbDir(), 'nwb-schema')); - supportedSchemas = setdiff({Schemas.name}, {'.', '..'}); - if ~any(strcmp(schemaVersion, supportedSchemas)) - warning('NWB:Read:UnsupportedSchema' ... - , ['NWB schema version %s is not support by this version of MatNWB. ' ... - 'This file is not guaranteed to be supported.'] ... - , schemaVersion); + regenerateSchemaClasses = not( any(strcmpi(string(flags), 'ignorecache')) ); + + schemaVersion = util.getSchemaVersion(filename); + try + matnwb.common.mustBeValidSchemaVersion(schemaVersion) + catch + warning('NWB:Read:UnsupportedSchema', ... + ['NWB schema version `%s` is not support by this version of MatNWB. ' ... + 'This file is not guaranteed to be supported.'], schemaVersion ) end - - if ~ignoreCache + + specLocation = io.spec.getEmbeddedSpecLocation(filename); + + if regenerateSchemaClasses if isempty(specLocation) try - generateCore(schemaVersion, 'savedir', saveDir); + generateCore(schemaVersion, 'savedir', options.savedir); catch ME - if ~strcmp(ME.identifier, 'NWB:GenerateCore:MissingCoreSchema') + if ~strcmp(ME.identifier, 'NWB:VersionValidator:UnsupportedSchemaVersion') rethrow(ME); end end else - generateSpec(filename, h5info(filename, specLocation), 'savedir', saveDir); + generateEmbeddedSpec(filename, specLocation, 'savedir', options.savedir); end rehash(); end - - nwb = io.parseGroup(filename, h5info(filename), Blacklist); -end -function specLocation = getEmbeddedSpec(filename) - specLocation = ''; - fid = H5F.open(filename); - try - %check for .specloc - attributeId = H5A.open(fid, '.specloc'); - referenceRawData = H5A.read(attributeId); - specLocation = H5R.get_name(attributeId, 'H5R_OBJECT', referenceRawData); - H5A.close(attributeId); - catch ME - if ~strcmp(ME.identifier, 'MATLAB:imagesci:hdf5lib:libraryError') - rethrow(ME); - end % don't error if the attribute doesn't exist. + blackList = struct(... + 'attributes', {{'.specloc', 'object_id'}},... + 'groups', {{}}); + if ~isempty(specLocation) + blackList.groups{end+1} = specLocation; end - H5F.close(fid); + nwb = io.parseGroup(filename, h5info(filename), blackList); end -function generateSpec(filename, specinfo, varargin) - saveDirMask = strcmp(varargin, 'savedir'); - if any(saveDirMask) - assert(~saveDirMask(end),... - 'NWB:Read:InvalidParameter',... - 'savedir must be paired with the desired save directory.'); - saveDir = varargin{find(saveDirMask, 1, 'last') + 1}; - else - saveDir = misc.getMatnwbDir(); + +function generateEmbeddedSpec(filename, specLocation, options) +% generateEmbeddedSpec - Generate embedded specifications / namespaces + arguments + filename (1,1) string {mustBeFile} + specLocation (1,1) string + options.savedir (1,1) string = misc.getMatnwbDir(); % {mustBeFolder} ? end - - specNames = cell(size(specinfo.Groups)); - fid = H5F.open(filename); - for iGroup = 1:length(specinfo.Groups) - location = specinfo.Groups(iGroup).Groups(1); - - namespaceName = split(specinfo.Groups(iGroup).Name, '/'); - namespaceName = namespaceName{end}; - - filenames = {location.Datasets.Name}; - if ~any(strcmp('namespace', filenames)) - warning('NWB:Read:GenerateSpec:CacheInvalid',... - 'Couldn''t find a `namespace` in namespace `%s`. Skipping cache generation.',... - namespaceName); - return; - end - sourceNames = {location.Datasets.Name}; - fileLocation = strcat(location.Name, '/', sourceNames); - schemaMap = containers.Map; - for iFileLocation = 1:length(fileLocation) - did = H5D.open(fid, fileLocation{iFileLocation}); - if strcmp('namespace', sourceNames{iFileLocation}) - namespaceText = H5D.read(did); - else - schemaMap(sourceNames{iFileLocation}) = H5D.read(did); - end - H5D.close(did); - end + + specs = io.spec.readEmbeddedSpecifications(filename, specLocation); + specNames = cell(size(specs)); + + for iSpec = 1:numel(specs) + namespaceName = specs{iSpec}.namespaceName; + namespaceDef = specs{iSpec}.namespaceText; + schemaMap = specs{iSpec}.schemaMap; + + parsedNamespace = spec.generate(namespaceDef, schemaMap); - Namespaces = spec.generate(namespaceText, schemaMap); - % Handle embedded namespaces. - Namespace = Namespaces(strcmp({Namespaces.name}, namespaceName)); - if isempty(Namespace) - % legacy checks in case namespaceName is using the old underscore - % conversion name. - namespaceName = strrep(namespaceName, '_', '-'); - Namespace = Namespaces(strcmp({Namespaces.name}, namespaceName)); + % Ensure the namespace name matches the name of the parsed namespace + isMatch = strcmp({parsedNamespace.name}, namespaceName); + if ~any(isMatch) % Legacy check + % Check if namespaceName is using the old underscore convention. + isMatch = strcmp({parsedNamespace.name}, strrep(namespaceName, '_', '-')); end - - assert(~isempty(Namespace), ... + + assert(any(isMatch), ... 'NWB:Namespace:NameNotFound', ... - 'Namespace %s not found in schema. Perhaps an extension should be generated?', ... + 'Namespace `%s` not found in specification. Perhaps an extension should be generated?', ... namespaceName); + + parsedNamespace = parsedNamespace(isMatch); - spec.saveCache(Namespace, saveDir); - specNames{iGroup} = Namespace.name; + spec.saveCache(parsedNamespace, options.savedir); + specNames{iSpec} = parsedNamespace.name; end - H5F.close(fid); missingNames = cell(size(specNames)); for iName = 1:length(specNames) name = specNames{iName}; try - file.writeNamespace(name, saveDir); + file.writeNamespace(name, options.savedir); catch ME + % Todo: Can this actually happen? if strcmp(ME.identifier, 'NWB:Namespace:CacheMissing') missingNames{iName} = name; else @@ -169,6 +116,7 @@ function generateSpec(filename, specinfo, varargin) end end end + missingNames(cellfun('isempty', missingNames)) = []; assert(isempty(missingNames), 'NWB:Namespace:DependencyMissing',... 'Missing generated caches and dependent caches for the following namespaces:\n%s',... From 0bec70d3b089536c6e1135f11df8803eeac55eac Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 7 Nov 2024 22:18:12 +0100 Subject: [PATCH 02/32] Update functionSignatures --- functionSignatures.json | 69 ++++++++++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 12 deletions(-) diff --git a/functionSignatures.json b/functionSignatures.json index 727d7537d..74f102d09 100644 --- a/functionSignatures.json +++ b/functionSignatures.json @@ -1,18 +1,45 @@ { + "_schemaVersion": "1.0.0", "generateCore": { "inputs": [ { - "name":"core namespace", - "kind":"optional", - "type":"filepath=*.yaml,*.yml" + "name":"version", + "kind":"required", + "type":"choices={'2.0.2','2.1.0','2.2.0','2.2.1','2.2.2','2.2.3','2.2.4','2.2.5','2.3.0','2.4.0','2.5.0','2.6.0','2.7.0'}", + "purpose": "Version number for NWB core schema specifications" }, { - "name":"extensions namespace(s)", - "kind":"optional", - "type":"filepath=*.yaml,*.yml", - "multiplicity":"append" + "name":"savedir", + "kind":"namevalue", + "type":"folder", + "purpose": "Output folder for generated classes" + } + ] + }, + "generateCore": + { + "inputs": + [ + { + "name":"savedir", + "kind":"namevalue", + "type":"folder", + "purpose": "Output folder for generated classes" + } + ] + }, + "generateExtension": + { + "inputs": + [ + { + "name":"namespaceFilePath", + "repeating":true, + "kind":"required", + "type":"file=*.yaml,*.yml", + "purpose": "Path to a *namespace.yaml file." } ] }, @@ -21,9 +48,17 @@ "inputs": [ { - "name":"namespace", + "name":"namespaceFilePath", + "repeating":true, "kind":"required", - "type":"filepath=*.yaml,*.yml" + "type":"file=*.yaml,*.yml", + "purpose": "Path to a *namespace.yaml file." + }, + { + "name":"savedir", + "kind":"namevalue", + "type":"folder", + "purpose": "Output folder for generated classes" } ] }, @@ -31,7 +66,17 @@ { "inputs": [ - {"name":"NWB File", "kind":"required", "type":"filepath=*.nwb,*.h5"} + {"name":"filename", "kind":"required", "type":"file=*.nwb,*.h5"}, + {"mutuallyExclusiveGroup": + [ + [ + {"name":"flag", "kind":"ordered", "type":"choices='ignorecache'"} + ], + [ + {"name":"savedir", "kind":"namevalue", "type":"folder","purpose": "Output folder for generated classes"} + ] + ] + } ], "outputs": [ @@ -42,8 +87,8 @@ { "inputs": [ - {"name":"NwbFile Object", "kind":"required", "type":"NwbFile"}, - {"name":"path to output file", "kind":"required", "type":"filepath=*.nwb"} + {"name":"nwbFileObjects", "kind":"required", "type":"NwbFile", "purpose":"An NWB file object or a list of NWB file objects"}, + {"name":"filePaths", "kind":"required", "type":"file=*.nwb", "purpose":"A filepath or a list of filepaths for exporting NWB file object(s)"} ] } } From 895fa1f53e6fc074b2d38428e50e5f8f88fb4ce9 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 7 Nov 2024 22:18:29 +0100 Subject: [PATCH 03/32] Create functionSignatures.json --- resources/functionSignatures.json | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 resources/functionSignatures.json diff --git a/resources/functionSignatures.json b/resources/functionSignatures.json new file mode 100644 index 000000000..c188cbf55 --- /dev/null +++ b/resources/functionSignatures.json @@ -0,0 +1,49 @@ +{ + "generateCore": + { + "inputs": + [ + { + "name":"version", + "kind":"ordered", + "type":"choices={'2.0.2','2.1.0','2.2.0','2.2.1','2.2.2','2.2.3','2.2.4','2.2.5','2.3.0','2.4.0','2.5.0','2.6.0','2.7.0'}" + }, + { + "name":"savedir", + "kind":"namevalue", + "type":"folder", + "purpose": "Output folder for generated classes" + } + ] + }, + "generateExtension": + { + "inputs": + [ + { + "name":"namespaceFilePath", + "kind":"required", + "type":"file=*.yaml,*.yml" + } + ] + }, + "nwbRead": + { + "inputs": + [ + {"name":"NWB File", "kind":"required", "type":"file=*.nwb,*.h5"} + ], + "outputs": + [ + {"name":"NwbFile Object", "type":"nwbfile"} + ] + }, + "nwbExport": + { + "inputs": + [ + {"name":"NwbFile Object", "kind":"required", "type":"NwbFile"}, + {"name":"path to output file", "kind":"required", "type":"file=*.nwb"} + ] + } +} From 39bb14770bba84fe366b63ba9041e8fa585c221a Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 7 Nov 2024 22:18:48 +0100 Subject: [PATCH 04/32] Improve coverage for main functions --- NwbFile.m | 23 +++++++++++++---------- generateCore.m | 2 +- generateExtension.m | 9 +++++---- nwbExport.m | 7 +++---- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/NwbFile.m b/NwbFile.m index f89ac708e..e7dcddbc0 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -124,16 +124,19 @@ function resolveReferences(obj, fid, references) resolved(iRef) = exportSuccess; end - if ~any(resolved) - errorFormat = ['object(s) could not be created:\n%s\n\nThe '... - 'listed object(s) above contain an ObjectView, '... - 'RegionView , or SoftLink object that has failed to resolve itself. '... - 'Please check for any references that were not assigned to the root '... - ' NwbFile or if any of the above paths are incorrect.']; - unresolvedRefs = strjoin(references, newline); - error('NWB:NwbFile:UnresolvedReferences',... - errorFormat, file.addSpaces(unresolvedRefs, 4)); - end + errorMessage = sprintf(... + ['Object(s) could not be created:\n%s\n\nThe listed '... + 'object(s) above contain an ObjectView, RegionView, or ' ... + 'SoftLink object that has failed to resolve itself. '... + 'Please check for any references that were not assigned ' ... + 'to the root NwbFile or if any of the above paths are ' ... + 'incorrect.'], file.addSpaces(strjoin(references, newline), 4)); + + assert( ... + all(resolved), ... + 'NWB:NwbFile:UnresolvedReferences', ... + errorMessage ... + ) references(resolved) = []; end diff --git a/generateCore.m b/generateCore.m index 03309e0f9..7349b8e16 100644 --- a/generateCore.m +++ b/generateCore.m @@ -25,7 +25,7 @@ function generateCore(version, options) arguments version (1,1) string {matnwb.common.mustBeValidSchemaVersion} = "latest" - options.savedir (1,1) string = missing + options.savedir (1,1) string = misc.getMatnwbDir() end if version == "latest" diff --git a/generateExtension.m b/generateExtension.m index 741264f69..f41843503 100644 --- a/generateExtension.m +++ b/generateExtension.m @@ -24,10 +24,11 @@ function generateExtension(namespaceFilePath, options) options.savedir (1,1) string = misc.getMatnwbDir() end - if isempty(namespaceFilePath) - error('NWB:GenerateExtension:NamespaceMissing', ... - 'Please provide the file path to at least one namespace specification file.') - end + assert( ... + ~isempty(namespaceFilePath), ... + 'NWB:GenerateExtension:NamespaceMissing', ... + 'Please provide the file path to at least one namespace specification file.' ... + ) for iNamespaceFiles = 1:length(namespaceFilePath) diff --git a/nwbExport.m b/nwbExport.m index 8f5a451f2..655d891d1 100644 --- a/nwbExport.m +++ b/nwbExport.m @@ -21,10 +21,9 @@ function nwbExport(nwbFileObjects, filePaths, mode) mode (1,1) string {mustBeMember(mode, ["edit", "overwrite"])} = "edit" end - if length(nwbFileObjects) ~= length(filePaths) - error('NWB:Export:FilepathLengthMismatch', ... - 'Lists of NWB objects to export and list of file paths must be the same length.') - end + assert(length(nwbFileObjects) == length(filePaths), ... + 'NWB:Export:FilepathLengthMismatch', ... + 'Lists of NWB objects to export and list of file paths must be the same length.') for iFiles = 1:length(nwbFileObjects) filePath = char(filePaths(iFiles)); From 1c1d7ec986a0492f363a60370fc9c6ccfc725e47 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 7 Nov 2024 23:03:39 +0100 Subject: [PATCH 05/32] Create TypeConversionTest.m --- +tests/+unit/+io/TypeConversionTest.m | 122 ++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 +tests/+unit/+io/TypeConversionTest.m diff --git a/+tests/+unit/+io/TypeConversionTest.m b/+tests/+unit/+io/TypeConversionTest.m new file mode 100644 index 000000000..44ab96e37 --- /dev/null +++ b/+tests/+unit/+io/TypeConversionTest.m @@ -0,0 +1,122 @@ +classdef TypeConversionTest < matlab.unittest.TestCase + % TypeConversionTest Unit test for io.getMatType function. + + properties (TestParameter) + matlabType = {... + 'types.untyped.ObjectView', ... + 'types.untyped.RegionView', ... + 'char', ... + 'double', ... + 'single', ... + 'logical', ... + 'int8', 'int16', 'int32', 'int64', ... + 'uint8', 'uint16', 'uint32', 'uint64', ... + } + end + + methods (Test) + + function testRoundTrip(testCase, matlabType) + tid = io.getBaseType(matlabType); + testCase.verifyEqual(io.getMatType(tid), matlabType); + end + + function testRoundTripCell(testCase) + tid = io.getBaseType('cell'); + testCase.verifyEqual(io.getMatType(tid), 'char'); + end + + function testRoundTripDatetime(testCase) + tid = io.getBaseType('datetime'); + testCase.verifyEqual(io.getMatType(tid), 'char'); + end + + function testRoundTripStruct(testCase) + testCase.verifyError(@(type)io.getBaseType('struct'), ''); + end + + function testDoubleType(testCase) + tid = H5T.copy('H5T_IEEE_F64LE'); + testCase.verifyEqual(io.getMatType(tid), 'double'); + end + + function testSingleType(testCase) + tid = H5T.copy('H5T_IEEE_F32LE'); + testCase.verifyEqual(io.getMatType(tid), 'single'); + end + + function testUint8Type(testCase) + tid = H5T.copy('H5T_STD_U8LE'); + testCase.verifyEqual(io.getMatType(tid), 'uint8'); + end + + function testInt8Type(testCase) + tid = H5T.copy('H5T_STD_I8LE'); + testCase.verifyEqual(io.getMatType(tid), 'int8'); + end + + function testUint16Type(testCase) + tid = H5T.copy('H5T_STD_U16LE'); + testCase.verifyEqual(io.getMatType(tid), 'uint16'); + end + + function testInt16Type(testCase) + tid = H5T.copy('H5T_STD_I16LE'); + testCase.verifyEqual(io.getMatType(tid), 'int16'); + end + + function testUint32Type(testCase) + tid = H5T.copy('H5T_STD_U32LE'); + testCase.verifyEqual(io.getMatType(tid), 'uint32'); + end + + function testInt32Type(testCase) + tid = H5T.copy('H5T_STD_I32LE'); + testCase.verifyEqual(io.getMatType(tid), 'int32'); + end + + function testUint64Type(testCase) + tid = H5T.copy('H5T_STD_U64LE'); + testCase.verifyEqual(io.getMatType(tid), 'uint64'); + end + + function testInt64Type(testCase) + tid = H5T.copy('H5T_STD_I64LE'); + testCase.verifyEqual(io.getMatType(tid), 'int64'); + end + + function testCharType(testCase) + tid = io.getBaseType('char'); % Assuming io.getBaseType exists + testCase.verifyEqual(io.getMatType(tid), 'char'); + end + + function testObjectViewType(testCase) + tid = H5T.copy('H5T_STD_REF_OBJ'); + testCase.verifyEqual(io.getMatType(tid), 'types.untyped.ObjectView'); + end + + function testRegionViewType(testCase) + tid = H5T.copy('H5T_STD_REF_DSETREG'); + testCase.verifyEqual(io.getMatType(tid), 'types.untyped.RegionView'); + end + + function testLogicalType(testCase) + % Simulate or define a logical type ID for testing + tid = H5T.enum_create('H5T_NATIVE_INT'); + H5T.enum_insert(tid, 'FALSE', 0); + H5T.enum_insert(tid, 'TRUE', 1); + + testCase.verifyEqual(io.getMatType(tid), 'logical'); + end + + function testTableType(testCase) + tid = H5T.create('H5T_COMPOUND', 10); + testCase.verifyEqual(io.getMatType(tid), 'table'); + end + + function testUnknownType(testCase) + tid = H5T.copy('H5T_NATIVE_B64'); % Example of an unknown type + testCase.verifyError(@() io.getMatType(tid), 'NWB:IO:GetMatlabType:UnknownTypeID'); + end + end +end From 2a31310cfdbd3aa25021469410d41967d82dd62e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 7 Nov 2024 23:22:54 +0100 Subject: [PATCH 06/32] Add unit tests for io.isBool and io.pathParts --- +tests/+unit/+io/IsBoolTest.m | 10 ++++++++++ +tests/+unit/+io/PathPartsTest.m | 24 ++++++++++++++++++++++++ +tests/+unit/+io/TypeConversionTest.m | 2 +- 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 +tests/+unit/+io/IsBoolTest.m create mode 100644 +tests/+unit/+io/PathPartsTest.m diff --git a/+tests/+unit/+io/IsBoolTest.m b/+tests/+unit/+io/IsBoolTest.m new file mode 100644 index 000000000..ada77b57b --- /dev/null +++ b/+tests/+unit/+io/IsBoolTest.m @@ -0,0 +1,10 @@ +classdef IsBoolTest < matlab.unittest.TestCase +% IsBoolTest - Unit test for io.isBool function. + + methods (Test) + function testInvalidInput(testCase) + testCase.verifyError(@(x) io.isBool("string"), ... + 'NWB:IO:IsBool:InvalidArgument') + end + end +end \ No newline at end of file diff --git a/+tests/+unit/+io/PathPartsTest.m b/+tests/+unit/+io/PathPartsTest.m new file mode 100644 index 000000000..7036a467e --- /dev/null +++ b/+tests/+unit/+io/PathPartsTest.m @@ -0,0 +1,24 @@ +classdef PathPartsTest < matlab.unittest.TestCase +% PathPartsTest - Unit test for io.pathParts function. + +% Todo: Function has confusing naming of outputs. Should be fixed + methods (Test) + function testRootPath(testCase) + [stem, root] = io.pathParts('root'); + testCase.verifyEqual(root, 'root') + testCase.verifyEmpty(stem) + end + + function testRootWithStemPath(testCase) + [stem, root] = io.pathParts('root/stem'); + testCase.verifyEqual(root, 'stem') + testCase.verifyEqual(stem, 'root') + end + + function testRootWithLongerStemPath(testCase) + [stem, root] = io.pathParts('root/stem/leaf'); + testCase.verifyEqual(root, 'leaf') + testCase.verifyEqual(stem, 'root/stem') + end + end +end \ No newline at end of file diff --git a/+tests/+unit/+io/TypeConversionTest.m b/+tests/+unit/+io/TypeConversionTest.m index 44ab96e37..65c0ecac8 100644 --- a/+tests/+unit/+io/TypeConversionTest.m +++ b/+tests/+unit/+io/TypeConversionTest.m @@ -1,5 +1,5 @@ classdef TypeConversionTest < matlab.unittest.TestCase - % TypeConversionTest Unit test for io.getMatType function. +% TypeConversionTest - Unit test for io.getMatType and io.getBaseType functions. properties (TestParameter) matlabType = {... From ff62e74605523d73c664bfc1e9a6af36d9d45b8b Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 00:20:23 +0100 Subject: [PATCH 07/32] Update functionSignatures --- functionSignatures.json | 94 ------------------------------- resources/functionSignatures.json | 44 +++++++++++++-- 2 files changed, 38 insertions(+), 100 deletions(-) delete mode 100644 functionSignatures.json diff --git a/functionSignatures.json b/functionSignatures.json deleted file mode 100644 index 74f102d09..000000000 --- a/functionSignatures.json +++ /dev/null @@ -1,94 +0,0 @@ -{ - "_schemaVersion": "1.0.0", - "generateCore": - { - "inputs": - [ - { - "name":"version", - "kind":"required", - "type":"choices={'2.0.2','2.1.0','2.2.0','2.2.1','2.2.2','2.2.3','2.2.4','2.2.5','2.3.0','2.4.0','2.5.0','2.6.0','2.7.0'}", - "purpose": "Version number for NWB core schema specifications" - }, - { - "name":"savedir", - "kind":"namevalue", - "type":"folder", - "purpose": "Output folder for generated classes" - } - ] - }, - "generateCore": - { - "inputs": - [ - { - "name":"savedir", - "kind":"namevalue", - "type":"folder", - "purpose": "Output folder for generated classes" - } - ] - }, - "generateExtension": - { - "inputs": - [ - { - "name":"namespaceFilePath", - "repeating":true, - "kind":"required", - "type":"file=*.yaml,*.yml", - "purpose": "Path to a *namespace.yaml file." - } - ] - }, - "generateExtension": - { - "inputs": - [ - { - "name":"namespaceFilePath", - "repeating":true, - "kind":"required", - "type":"file=*.yaml,*.yml", - "purpose": "Path to a *namespace.yaml file." - }, - { - "name":"savedir", - "kind":"namevalue", - "type":"folder", - "purpose": "Output folder for generated classes" - } - ] - }, - "nwbRead": - { - "inputs": - [ - {"name":"filename", "kind":"required", "type":"file=*.nwb,*.h5"}, - {"mutuallyExclusiveGroup": - [ - [ - {"name":"flag", "kind":"ordered", "type":"choices='ignorecache'"} - ], - [ - {"name":"savedir", "kind":"namevalue", "type":"folder","purpose": "Output folder for generated classes"} - ] - ] - } - ], - "outputs": - [ - {"name":"NwbFile Object", "type":"nwbfile"} - ] - }, - "nwbExport": - { - "inputs": - [ - {"name":"nwbFileObjects", "kind":"required", "type":"NwbFile", "purpose":"An NWB file object or a list of NWB file objects"}, - {"name":"filePaths", "kind":"required", "type":"file=*.nwb", "purpose":"A filepath or a list of filepaths for exporting NWB file object(s)"} - ] - } -} diff --git a/resources/functionSignatures.json b/resources/functionSignatures.json index c188cbf55..cf52fb2cc 100644 --- a/resources/functionSignatures.json +++ b/resources/functionSignatures.json @@ -1,12 +1,14 @@ { + "_schemaVersion": "1.0.0", "generateCore": { "inputs": [ { "name":"version", - "kind":"ordered", - "type":"choices={'2.0.2','2.1.0','2.2.0','2.2.1','2.2.2','2.2.3','2.2.4','2.2.5','2.3.0','2.4.0','2.5.0','2.6.0','2.7.0'}" + "kind":"required", + "type":"choices={'2.0.2','2.1.0','2.2.0','2.2.1','2.2.2','2.2.3','2.2.4','2.2.5','2.3.0','2.4.0','2.5.0','2.6.0','2.7.0'}", + "purpose": "Version number for NWB core schema specifications" }, { "name":"savedir", @@ -16,14 +18,34 @@ } ] }, + "generateCore": + { + "inputs": + [ + { + "name":"savedir", + "kind":"namevalue", + "type":"folder", + "purpose": "Output folder for generated classes" + } + ] + }, "generateExtension": { "inputs": [ { "name":"namespaceFilePath", + "repeating":true, "kind":"required", - "type":"file=*.yaml,*.yml" + "type":"file=*.yaml,*.yml", + "purpose": "Path to a *namespace.yaml file." + }, + { + "name":"savedir", + "kind":"namevalue", + "type":"folder", + "purpose": "Output folder for generated classes" } ] }, @@ -31,7 +53,17 @@ { "inputs": [ - {"name":"NWB File", "kind":"required", "type":"file=*.nwb,*.h5"} + {"name":"filename", "kind":"required", "type":"file=*.nwb,*.h5"}, + {"mutuallyExclusiveGroup": + [ + [ + {"name":"flag", "kind":"ordered", "type":"choices='ignorecache'"} + ], + [ + {"name":"savedir", "kind":"namevalue", "type":"folder","purpose": "Output folder for generated classes"} + ] + ] + } ], "outputs": [ @@ -42,8 +74,8 @@ { "inputs": [ - {"name":"NwbFile Object", "kind":"required", "type":"NwbFile"}, - {"name":"path to output file", "kind":"required", "type":"file=*.nwb"} + {"name":"nwbFileObjects", "kind":"required", "type":"NwbFile", "purpose":"An NWB file object or a list of NWB file objects"}, + {"name":"filePaths", "kind":"required", "type":"file=*.nwb", "purpose":"A filepath or a list of filepaths for exporting NWB file object(s)"} ] } } From ef2c5926b163c64d0893c1a88dc91775f6aafb33 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 17:25:59 +0100 Subject: [PATCH 08/32] Add test for cloneNwbFileClass --- +file/cloneNwbFileClass.m | 19 ++++++++---- +io/parseGroup.m | 2 -- +tests/+unit/+file/CloneNwbTest.m | 50 +++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 +tests/+unit/+file/CloneNwbTest.m diff --git a/+file/cloneNwbFileClass.m b/+file/cloneNwbFileClass.m index c62b25d0b..027718494 100644 --- a/+file/cloneNwbFileClass.m +++ b/+file/cloneNwbFileClass.m @@ -5,14 +5,21 @@ function cloneNwbFileClass(typeFileName, fullTypeName) nwbFilePath = which('NwbFile'); installPath = fileparts(nwbFilePath); -fileId = fopen(nwbFilePath); -text = strrep(char(fread(fileId) .'),... - 'NwbFile < types.core.NWBFile',... +nwbFileClassDef = fileread(nwbFilePath); + +% Update superclass name +updatedNwbFileClassDef = strrep(nwbFileClassDef, ... + 'NwbFile < types.core.NWBFile', ... sprintf('NwbFile < %s', fullTypeName)); -fclose(fileId); + +% Update call to superclass constructor +updatedNwbFileClassDef = strrep(updatedNwbFileClassDef, ... + 'obj = obj@types.core.NWBFile', ... + sprintf('obj = obj@%s', fullTypeName)); fileId = fopen(fullfile(installPath, [typeFileName '.m']), 'W'); -fwrite(fileId, text); +fwrite(fileId, updatedNwbFileClassDef); fclose(fileId); -end +rehash(); +end diff --git a/+io/parseGroup.m b/+io/parseGroup.m index 791f2ecf7..4574e8d75 100644 --- a/+io/parseGroup.m +++ b/+io/parseGroup.m @@ -76,9 +76,7 @@ parsed = NwbFile(kwargs{:}); else file.cloneNwbFileClass(Type.name, Type.typename); - rehash(); parsed = io.createParsedType(info.Name, Type.typename, kwargs{:}); - end return; diff --git a/+tests/+unit/+file/CloneNwbTest.m b/+tests/+unit/+file/CloneNwbTest.m new file mode 100644 index 000000000..de6cea2d2 --- /dev/null +++ b/+tests/+unit/+file/CloneNwbTest.m @@ -0,0 +1,50 @@ +classdef CloneNwbTest < matlab.unittest.TestCase + + methods (TestClassSetup) + function setupClass(testCase) + % Get the root path of the matnwb repository + rootPath = misc.getMatnwbDir(); + + % Use a fixture to add the folder to the search path + testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath)); + + % Use a fixture to create a temporary working directory + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + end + end + + methods (Test) + function testCloneNwbFile(testCase) + % Create a superclass + superClassDef = [... + 'classdef MyCustomNwbFile < types.core.NWBFile\n', ... + ' methods\n', ... + ' function sayHello(obj)\n', ... + ' fprintf(''Hello %%s\\n'', obj.general_experimenter)\n', ... + ' end\n', ... + ' end\n', ... + 'end\n']; + fid = fopen('MyCustomNwbFile.m', 'w'); + fprintf(fid, superClassDef); + fclose(fid); + + currentClassDef = fileread(fullfile(misc.getMatnwbDir(), 'NwbFile.m')); + cleanupObj = onCleanup(@(classDefStr) restoreNwbFileClass(currentClassDef)); + + file.cloneNwbFileClass(fullfile('NwbFile'), 'MyCustomNwbFile') + + testCase.verifyTrue( isfile(fullfile(misc.getMatnwbDir(), 'NwbFile.m')) ) + + nwbFile = NwbFile(); + nwbFile.general_experimenter = "Mouse McMouse"; + C = evalc('nwbFile.sayHello()'); + testCase.verifyEqual(C, sprintf('Hello Mouse McMouse\n')); + end + end +end + +function restoreNwbFileClass(classDefStr) + fid = fopen( fullfile(misc.getMatnwbDir(), 'NwbFile.m'), 'wt' ); + fwrite(fid, classDefStr); + fclose(fid); +end \ No newline at end of file From 0c0671f3cf9c3e398b3e915ff1761e85f3856a8a Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 18:32:13 +0100 Subject: [PATCH 09/32] Create local validator function for reftype in file.fillProps - Simplify logic in fillExport --- +file/fillExport.m | 10 ++---- +file/fillProps.m | 40 ++++++++++++------------ +types/+hdmf_common/DynamicTableRegion.m | 2 +- +types/+hdmf_common/VectorIndex.m | 2 +- +types/+hdmf_experimental/EnumData.m | 2 +- 5 files changed, 26 insertions(+), 30 deletions(-) diff --git a/+file/fillExport.m b/+file/fillExport.m index 9a4a242c5..589fc0a35 100644 --- a/+file/fillExport.m +++ b/+file/fillExport.m @@ -21,9 +21,6 @@ for i = 1:length(propertyNames) propertyName = propertyNames{i}; pathProps = traverseRaw(propertyName, RawClass); - if isempty(pathProps) - keyboard; - end prop = pathProps{end}; elideProps = pathProps(1:end-1); elisions = cell(length(elideProps),1); @@ -84,11 +81,10 @@ path = {}; if isa(RawClass, 'file.Dataset') - if isempty(RawClass.attributes) - return; + if ~isempty(RawClass.attributes) + matchesAttribute = strcmp({RawClass.attributes.name}, propertyName); + path = {RawClass.attributes(matchesAttribute)}; end - matchesAttribute = strcmp({RawClass.attributes.name}, propertyName); - path = {RawClass.attributes(matchesAttribute)}; return; end diff --git a/+file/fillProps.m b/+file/fillProps.m index 51062cd22..47faf2166 100644 --- a/+file/fillProps.m +++ b/+file/fillProps.m @@ -52,30 +52,14 @@ typeStr = ['Table with columns: (', strjoin(columnDocStr, ', '), ')']; elseif isa(prop, 'file.Attribute') if isa(prop.dtype, 'containers.Map') - switch prop.dtype('reftype') - case 'region' - refTypeName = 'Region'; - case 'object' - refTypeName = 'Object'; - otherwise - error('NWB:ClassGenerator:InvalidRefType', ... - 'Invalid reftype found while filling description for class property "%s".', propName); - end - typeStr = sprintf('%s Reference to %s', refTypeName, prop.dtype('target_type')); + assertValidRefType(prop.dtype('reftype')) + typeStr = sprintf('%s reference to %s', capitalize(prop.dtype('reftype')), prop.dtype('target_type')); else typeStr = prop.dtype; end elseif isa(prop, 'containers.Map') - switch prop('reftype') - case 'region' - refTypeName = 'region'; - case 'object' - refTypeName = 'object'; - otherwise - error('NWB:ClassGenerator:InvalidRefType', ... - 'Invalid reftype found while filling description for class property "%s".', propName); - end - typeStr = sprintf('%s Reference to %s', refTypeName, prop('target_type')); + assertValidRefType(prop('reftype')) + typeStr = sprintf('%s reference to %s', capitalize(prop('reftype')), prop('target_type')); elseif isa(prop, 'file.interface.HasProps') typeStrCell = cell(size(prop)); for iProp = 1:length(typeStrCell) @@ -108,4 +92,20 @@ if nargin >= 2 propStr = [propName ' = ' propStr]; end +end + +function assertValidRefType(referenceType) + arguments + referenceType (1,1) string + end + assert( ismember(referenceType, ["region", "object"]), ... + 'NWB:ClassGenerator:InvalidRefType', ... + 'Invalid reftype found while filling description for class properties.') +end + +function word = capitalize(word) + arguments + word (1,:) char + end + word(1) = upper(word(1)); end \ No newline at end of file diff --git a/+types/+hdmf_common/DynamicTableRegion.m b/+types/+hdmf_common/DynamicTableRegion.m index e24dc59ee..506b6b023 100644 --- a/+types/+hdmf_common/DynamicTableRegion.m +++ b/+types/+hdmf_common/DynamicTableRegion.m @@ -4,7 +4,7 @@ % OPTIONAL PROPERTIES properties - table; % (Object Reference to DynamicTable) Reference to the DynamicTable object that this region applies to. + table; % (Object reference to DynamicTable) Reference to the DynamicTable object that this region applies to. end methods diff --git a/+types/+hdmf_common/VectorIndex.m b/+types/+hdmf_common/VectorIndex.m index 75a9e683b..4a40a9363 100644 --- a/+types/+hdmf_common/VectorIndex.m +++ b/+types/+hdmf_common/VectorIndex.m @@ -4,7 +4,7 @@ % OPTIONAL PROPERTIES properties - target; % (Object Reference to VectorData) Reference to the target dataset that this index applies to. + target; % (Object reference to VectorData) Reference to the target dataset that this index applies to. end methods diff --git a/+types/+hdmf_experimental/EnumData.m b/+types/+hdmf_experimental/EnumData.m index 1608dea02..e22843e10 100644 --- a/+types/+hdmf_experimental/EnumData.m +++ b/+types/+hdmf_experimental/EnumData.m @@ -4,7 +4,7 @@ % OPTIONAL PROPERTIES properties - elements; % (Object Reference to VectorData) Reference to the VectorData object that contains the enumerable elements + elements; % (Object reference to VectorData) Reference to the VectorData object that contains the enumerable elements end methods From a2ac9f0a800014dfcd233d5d8980003c596637c3 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 18:46:01 +0100 Subject: [PATCH 10/32] Create local function for duplicated code block --- +file/fillValidators.m | 44 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/+file/fillValidators.m b/+file/fillValidators.m index e2daac36e..57d474e70 100644 --- a/+file/fillValidators.m +++ b/+file/fillValidators.m @@ -155,34 +155,19 @@ fillDimensionValidation(prop.dtype, prop.shape)... }, newline); elseif prop.isConstrainedSet - try - fullname = namespaceReg.getFullClassName(prop.type); - catch ME - if ~endsWith(ME.identifier, 'Namespace:NotFound') - rethrow(ME); - end - - warning('NWB:Fill:Validators:NamespaceNotFound',... - ['Namespace could not be found for type `%s`.' ... - ' Skipping Validation for property `%s`.'], prop.type, name); - return; + fullname = getFullClassName(namespaceReg, prop.type, name); + if isempty(fullname) + return end + unitValidationStr = strjoin({unitValidationStr... ['constrained = { ''' fullname ''' };']... ['types.util.checkSet(''' name ''', struct(), constrained, val);']... }, newline); else - try - fullname = namespaceReg.getFullClassName(prop.type); - catch ME - if ~endsWith(ME.identifier, 'Namespace:NotFound') - rethrow(ME); - end - - warning('NWB:Fill:Validators:NamespaceNotFound',... - ['Namespace could not be found for type `%s`.' ... - ' Skipping Validation for property `%s`.'], prop.type, name); - return; + fullname = getFullClassName(namespaceReg, prop.type, name); + if isempty(fullname) + return end unitValidationStr = [unitValidationStr newline fillDtypeValidation(name, fullname)]; end @@ -318,4 +303,19 @@ condition, ... sprintf(' %s', errorStr), ... 'end' }, newline ); +end + +function fullname = getFullClassName(namespaceReg, propType, name) + fullname = ''; + try + fullname = namespaceReg.getFullClassName(propType); + catch ME + if ~endsWith(ME.identifier, 'Namespace:NotFound') + rethrow(ME); + end + + warning('NWB:Fill:Validators:NamespaceNotFound',... + ['Namespace could not be found for type `%s`.' ... + ' Skipping Validation for property `%s`.'], propType, name); + end end \ No newline at end of file From cd216828220e1c89dfe59179e40b3ecb09dbcc9d Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 18:58:24 +0100 Subject: [PATCH 11/32] Simplify file cleanup in case of error --- +file/writeNamespace.m | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/+file/writeNamespace.m b/+file/writeNamespace.m index 10e6ef537..abe62578f 100644 --- a/+file/writeNamespace.m +++ b/+file/writeNamespace.m @@ -19,13 +19,9 @@ function writeNamespace(namespaceName, saveDir) end fid = fopen(fullfile(classFileDir, [className '.m']), 'W'); - try - fwrite(fid, file.fillClass(className, Namespace, processed, ... - classprops, inherited), 'char'); - catch ME - fclose(fid); - rethrow(ME) - end - fclose(fid); + % Create cleanup object to close to file in case the write operation fails. + fileCleanupObj = onCleanup(@(id) fclose(fid)); + fwrite(fid, file.fillClass(className, Namespace, processed, ... + classprops, inherited), 'char'); end end \ No newline at end of file From ef99eb942ce97a6672bb38db77b1419f7b5eb6ce Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 19:01:28 +0100 Subject: [PATCH 12/32] Update writeNamespace.m continue is unecessary here --- +file/writeNamespace.m | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/+file/writeNamespace.m b/+file/writeNamespace.m index abe62578f..00ab7aa91 100644 --- a/+file/writeNamespace.m +++ b/+file/writeNamespace.m @@ -14,14 +14,14 @@ function writeNamespace(namespaceName, saveDir) className = classes{i}; [processed, classprops, inherited] = file.processClass(className, Namespace, pregenerated); - if isempty(processed) - continue; + if ~isempty(processed) + fid = fopen(fullfile(classFileDir, [className '.m']), 'W'); + % Create cleanup object to close to file in case the write operation fails. + fileCleanupObj = onCleanup(@(id) fclose(fid)); + fwrite(fid, file.fillClass(className, Namespace, processed, ... + classprops, inherited), 'char'); + else + % pass end - - fid = fopen(fullfile(classFileDir, [className '.m']), 'W'); - % Create cleanup object to close to file in case the write operation fails. - fileCleanupObj = onCleanup(@(id) fclose(fid)); - fwrite(fid, file.fillClass(className, Namespace, processed, ... - classprops, inherited), 'char'); end end \ No newline at end of file From a78a2394c5863dc5de478c1eef7415e098cd18e7 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 19:09:13 +0100 Subject: [PATCH 13/32] Fix failing tests --- +tests/+unit/+file/CloneNwbTest.m | 2 ++ +tests/+unit/+io/TypeConversionTest.m | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/+tests/+unit/+file/CloneNwbTest.m b/+tests/+unit/+file/CloneNwbTest.m index de6cea2d2..1aeb11ab3 100644 --- a/+tests/+unit/+file/CloneNwbTest.m +++ b/+tests/+unit/+file/CloneNwbTest.m @@ -10,6 +10,8 @@ function setupClass(testCase) % Use a fixture to create a temporary working directory testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + + generateCore('savedir', '.') end end diff --git a/+tests/+unit/+io/TypeConversionTest.m b/+tests/+unit/+io/TypeConversionTest.m index 65c0ecac8..7438289c4 100644 --- a/+tests/+unit/+io/TypeConversionTest.m +++ b/+tests/+unit/+io/TypeConversionTest.m @@ -32,7 +32,8 @@ function testRoundTripDatetime(testCase) end function testRoundTripStruct(testCase) - testCase.verifyError(@(type)io.getBaseType('struct'), ''); + testCase.verifyError(@(type)io.getBaseType('struct'), ... + 'NWB:IO:UnsupportedBaseType'); end function testDoubleType(testCase) From 29a3021c9a473eebbc677c1f92322a94dc936952 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 21:25:06 +0100 Subject: [PATCH 14/32] Improve coverage --- +schemes/Namespace.m | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/+schemes/Namespace.m b/+schemes/Namespace.m index 6899f5429..72a8c7e04 100644 --- a/+schemes/Namespace.m +++ b/+schemes/Namespace.m @@ -1,8 +1,8 @@ classdef Namespace < handle properties (SetAccess=private) - name; %name of this namespace - dependencies; %parent namespaces by [Namespace] - registry; %maps name to class + name = '' % name of this namespace + dependencies = [] % parent namespaces by [Namespace] + registry = [] % maps name to class end properties (Constant) @@ -13,10 +13,7 @@ methods function obj = Namespace(name, deplist, source) if nargin == 0 - obj.name = ''; - obj.dependencies = []; - obj.registry = []; - return; + return end obj.name = strrep(name, '-', '_'); @@ -40,9 +37,12 @@ function parent = getParent(obj, classname) class = obj.getClass(classname); - if isempty(class) - error('NWB:Namespace:ClassNotFound', 'Could not find class %s', classname); - end + + assert( ... + ~isempty(class), ... + 'NWB:Namespace:ClassNotFound', ... + 'Could not find class %s', classname ... + ); parent = []; hasParentKey = isKey(class, obj.PARENT_KEYS); From efafb5325aa98a01df5c84920a329cee6e1ef06d Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 21:25:20 +0100 Subject: [PATCH 15/32] Add tests for misc functions --- +tests/+unit/+common/ValidatorTest.m | 10 ++++++++++ +tests/+unit/FunctionTests.m | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 +tests/+unit/+common/ValidatorTest.m create mode 100644 +tests/+unit/FunctionTests.m diff --git a/+tests/+unit/+common/ValidatorTest.m b/+tests/+unit/+common/ValidatorTest.m new file mode 100644 index 000000000..ab59a29c3 --- /dev/null +++ b/+tests/+unit/+common/ValidatorTest.m @@ -0,0 +1,10 @@ +classdef ValidatorTest < matlab.unittest.TestCase +% ValidatorTest - Unit test for validators. + + methods (Test) + function testInvalidVersionNumberFormat(testCase) testCase.verifyError( ... + @(vn) matnwb.common.mustBeValidSchemaVersion('1.0'), ... + 'NWB:VersionValidator:InvalidVersionNumber') + end + end +end \ No newline at end of file diff --git a/+tests/+unit/FunctionTests.m b/+tests/+unit/FunctionTests.m new file mode 100644 index 000000000..e7e74b92c --- /dev/null +++ b/+tests/+unit/FunctionTests.m @@ -0,0 +1,16 @@ +classdef FunctionTests < matlab.unittest.TestCase +% FunctionTests - Unit test for functions. + + methods (Test) + function testString2ValidName(testCase) + testCase.verifyWarning( ... + @(n,p) misc.str2validName('Time-Series', "test-a"), ... + 'NWB:CreateValidPropertyName:InvalidPrefix' ) + + validName = misc.str2validName('@id', 'at'); + testCase.verifyEqual(string(validName), "at_id") + end + + + end +end \ No newline at end of file From 5429e56ae16819aaab0a8f00536b7fd1ee2f9c54 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 8 Nov 2024 21:45:11 +0100 Subject: [PATCH 16/32] Simplify spec.loadCache Add arguments block --- +spec/loadCache.m | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/+spec/loadCache.m b/+spec/loadCache.m index e448735da..d13cbe45e 100644 --- a/+spec/loadCache.m +++ b/+spec/loadCache.m @@ -1,40 +1,34 @@ -function Cache = loadCache(varargin) +function Cache = loadCache(namespaceName, options) %LOADCACHE Loads Raw Namespace Metadata from cached directory -saveDirMask = strcmp(varargin, 'savedir'); -if any(saveDirMask) - assert(~saveDirMask(end),... - 'NWB:LoadCache:InvalidParameter',... - 'savedir must be paired with the desired save directory.'); - saveDir = varargin{find(saveDirMask, 1, 'last') + 1}; - saveDirParametersMask = saveDirMask | circshift(saveDirMask, 1); - namespaceList = varargin(~saveDirParametersMask); -else - saveDir = misc.getMatnwbDir(); - namespaceList = varargin; +arguments (Repeating) + namespaceName (1,1) string end +arguments + options.savedir (1,1) string = misc.getMatnwbDir() +end + +Cache = struct.empty; % Initialize output + +namespaceList = string(namespaceName); % Get the actual location of the matnwb directory. -namespaceDir = fullfile(saveDir, 'namespaces'); +namespaceDir = fullfile(options.savedir, 'namespaces'); fileList = dir(namespaceDir); fileList = fileList(~[fileList.isdir]); -if nargin > 0 - assert(iscellstr(namespaceList), 'Input arguments must be a list of namespace names.'); +if ~isempty(namespaceList) names = {fileList.name}; - whitelistIdx = ismember(names, strcat(namespaceList, '.mat')); + whitelistIdx = ismember(names, strcat(namespaceList + ".mat")); fileList = fileList(whitelistIdx); end -if isempty(fileList) - Cache = struct([]); - return; +if ~isempty(fileList) + matPath = fullfile(namespaceDir, fileList(1).name); + Cache = load(matPath); % initialize Cache first + for iMat = 2:length(fileList) + matPath = fullfile(namespaceDir, fileList(iMat).name); + Cache(iMat) = load(matPath); + end end - -matPath = fullfile(namespaceDir, fileList(1).name); -Cache = load(matPath); % initialize Cache first -for iMat = 2:length(fileList) - matPath = fullfile(namespaceDir, fileList(iMat).name); - Cache(iMat) = load(matPath); end -end \ No newline at end of file From d92dacb54130bbcb7c464f7849512a2599d66dc1 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 9 Nov 2024 21:01:10 +0100 Subject: [PATCH 17/32] Remove unused function --- +types/+untyped/Set.m | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/+types/+untyped/Set.m b/+types/+untyped/Set.m index 0fb36837f..4790f7623 100644 --- a/+types/+untyped/Set.m +++ b/+types/+untyped/Set.m @@ -220,19 +220,4 @@ function displayNonScalarObject(obj) 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 - end - end end \ No newline at end of file From ffdc5b89114ba9e4519c45a921c3f43ddc877fda Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 9 Nov 2024 21:01:38 +0100 Subject: [PATCH 18/32] Add misc unittests --- +tests/+system/NWBFileIOTest.m | 7 +++++++ +tests/+unit/FunctionTests.m | 24 +++++++++++++++++++++- +tests/+unit/dataPipeTest.m | 37 ++++++++++++++++++++++++++++++++++ +tests/+unit/untypedSetTest.m | 8 +++++++- 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/+tests/+system/NWBFileIOTest.m b/+tests/+system/NWBFileIOTest.m index f9747acde..03a382559 100644 --- a/+tests/+system/NWBFileIOTest.m +++ b/+tests/+system/NWBFileIOTest.m @@ -40,6 +40,13 @@ function writeMultipleFiles(testCase) nwbExport([fileA, fileB], {fileNameA, fileNameB}); end + function testLoadAll(testCase) + fileName = ['MatNWB.' testCase.className() '.testLoadAll.nwb']; + nwbExport(testCase.file, fileName) + nwb = nwbRead(fileName, "ignorecache"); + nwb.loadAll() + end + function readWithStringArg(testCase) fileName = ['MatNWB.' testCase.className() '.testReadWithStringArg.nwb']; fileName = string(fileName); diff --git a/+tests/+unit/FunctionTests.m b/+tests/+unit/FunctionTests.m index e7e74b92c..4b16b445e 100644 --- a/+tests/+unit/FunctionTests.m +++ b/+tests/+unit/FunctionTests.m @@ -11,6 +11,28 @@ function testString2ValidName(testCase) testCase.verifyEqual(string(validName), "at_id") end - + function testWriteCompoundMap(testCase) + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture) + fid = H5F.create('test.h5'); + data = containers.Map({'a', 'b'}, 1:2); + io.writeCompound(fid, '/map_data', data) + H5F.close(fid); + end + function testWriteCompoundEmpty(testCase) + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture) + fid = H5F.create('test.h5'); + data = struct; + testCase.verifyError(... + @(varargin) io.writeCompound(fid, '/map_data', data), ... + 'MATLAB:imagesci:hdf5lib:libraryError') + H5F.close(fid); + end + function testWriteCompoundScalar(testCase) + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture) + fid = H5F.create('test.h5'); + data = struct('a','b'); + io.writeCompound(fid, '/map_data', data) + H5F.close(fid); + end end end \ No newline at end of file diff --git a/+tests/+unit/dataPipeTest.m b/+tests/+unit/dataPipeTest.m index 639704725..be91df2e2 100644 --- a/+tests/+unit/dataPipeTest.m +++ b/+tests/+unit/dataPipeTest.m @@ -260,6 +260,43 @@ function testConfigurationFromData(testCase) testCase.verifyClass(conf, 'types.untyped.datapipe.Configuration') end +function testPropertySetGet(testCase) + data = rand(100, 1); + pipe = types.untyped.DataPipe('data', data); + + pipe.axis = 1; + testCase.verifyEqual(pipe.axis, 1) + + pipe.offset = 4; + testCase.verifyEqual(pipe.offset, 4) + + pipe.dataType = 'double'; + testCase.verifyEqual(pipe.dataType, 'double') + + pipe.chunkSize = 10; + testCase.verifyEqual(pipe.chunkSize, 10) + + pipe.compressionLevel = -1; + % Todo: make verification + + pipe.hasShuffle = false; + testCase.verifyFalse(pipe.hasShuffle) + + pipe.hasShuffle = true; + testCase.verifyTrue(pipe.hasShuffle) +end + +function testSubsrefWithNonScalarSubs(testCase) + data = rand(100, 1); + pipe = types.untyped.DataPipe('data', data); + + % This syntax should not be supported. Not clear what a valid + % non-scalar subsref would be... + subData = pipe{1:10}(1:5); + testCase.verifyEqual(subData, data(1:5)) +end + function data = createData(dataType, size) data = randi(intmax(dataType), size, dataType); end + diff --git a/+tests/+unit/untypedSetTest.m b/+tests/+unit/untypedSetTest.m index 4d18716fe..3fc9413f9 100644 --- a/+tests/+unit/untypedSetTest.m +++ b/+tests/+unit/untypedSetTest.m @@ -37,7 +37,7 @@ function testDisplayEmptyObject(testCase) end function testDisplayScalarObject(testCase) - scalarSet = types.untyped.Set('a',1) + scalarSet = types.untyped.Set('a',1); disp(scalarSet) end @@ -65,4 +65,10 @@ function testVerticalConcatenation(testCase) untypedSetB = types.untyped.Set( struct('c',3, 'd', 3) ); testCase.verifyError(@() [untypedSetA; untypedSetB], 'NWB:Set:Unsupported') +end + +function testSetCharValue(testCase) + untypedSet = types.untyped.Set( struct('a', 'a', 'b', 'b') ); + untypedSet.set('c', 'c') + testCase.verifyEqual(untypedSet.get('c'), 'c') end \ No newline at end of file From 79f260301b360e120530d001f28144eb73d856ba Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 10 Nov 2024 10:57:30 +0100 Subject: [PATCH 19/32] Add datapipe test and fix bug in DynamicFilter class --- +tests/+unit/dataPipeTest.m | 98 +++++++++++++++++-- .../+datapipe/+properties/DynamicFilter.m | 20 ++-- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/+tests/+unit/dataPipeTest.m b/+tests/+unit/dataPipeTest.m index be91df2e2..724e4937f 100644 --- a/+tests/+unit/dataPipeTest.m +++ b/+tests/+unit/dataPipeTest.m @@ -15,16 +15,22 @@ function setup(testCase) function testInit(testCase) import types.untyped.datapipe.*; - + import matlab.unittest.fixtures.SuppressedWarningsFixture + %testCase.applyFixture(SuppressedWarningsFixture('NWB:DataPipeTest:Debug')) + warnDebugId = 'NWB:DataPipeTest:Debug'; warning('off', warnDebugId); warning(warnDebugId, ''); %% extra data type data = rand(100, 1); - types.untyped.DataPipe('data', data, 'dataType', 'double'); - [~,lastId] = lastwarn(); - testCase.verifyEqual(lastId, 'NWB:DataPipe:RedundantDataType'); + % types.untyped.DataPipe('data', data, 'dataType', 'double'); + % [~,lastId] = lastwarn(); + % testCase.verifyEqual(lastId, 'NWB:DataPipe:RedundantDataType'); + + testCase.verifyWarning(... + @(varargin) types.untyped.DataPipe('data', data, 'dataType', 'double'), ... + 'NWB:DataPipe:RedundantDataType') warning(warnDebugId, ''); @@ -51,9 +57,14 @@ function testInit(testCase) pipe.export(fid, datasetName, {}); H5F.close(fid); + testCase.verifyWarning(... + @(varargin) types.untyped.DataPipe('filename', filename, 'path', datasetName, 'dataType', 'double'), ... + 'NWB:DataPipe:UnusedArguments') + + testCase.applyFixture(SuppressedWarningsFixture('NWB:DataPipe:UnusedArguments')) pipe = types.untyped.DataPipe('filename', filename, 'path', datasetName, 'dataType', 'double'); - [~,lastId] = lastwarn(); - testCase.verifyEqual(lastId, 'NWB:DataPipe:UnusedArguments'); + % [~,lastId] = lastwarn(); + % testCase.verifyEqual(lastId, 'NWB:DataPipe:UnusedArguments'); testCase.verifyEqual(pipe.compressionLevel, 2); testCase.verifyTrue(pipe.hasShuffle); @@ -131,6 +142,9 @@ function testExternalFilters(testCase) import types.untyped.datapipe.properties.DynamicFilter; import types.untyped.datapipe.properties.Shuffle; + % TODO: Why is Filter.LZ4 not part of the exported Pipe, i.e when the + % Pipe.internal goes from Blueprint to Bound + testCase.assumeTrue(logical(H5Z.filter_avail(uint32(Filter.LZ4)))); filename = 'testExternalWrite.h5'; @@ -153,7 +167,7 @@ function testExternalFilters(testCase) OneDimensionPipe.export(fid, '/test_one_dim_data', {}); H5F.close(fid); - + %% append data totalLength = 3; appendData = zeros([10 13 totalLength], Pipe.dataType); @@ -286,6 +300,28 @@ function testPropertySetGet(testCase) testCase.verifyTrue(pipe.hasShuffle) end +function testAppendVectorToBlueprintPipe(testCase) + % Column vector: + data = rand(10, 1); + pipe = types.untyped.DataPipe('data', data); + + pipe.append([1;2]); + newData = pipe.load(); + testCase.verifyEqual(newData, cat(1, data, [1;2])) + + testCase.verifyError(@(X) pipe.append([1,2]), 'MATLAB:catenate:dimensionMismatch') + + % Row vector: + data = rand(1, 10); + pipe = types.untyped.DataPipe('data', data); + + pipe.append([1,2]); + newData = pipe.load(); + testCase.verifyEqual(newData, cat(2, data, [1,2])) + + testCase.verifyError(@(X) pipe.append([1;2]), 'MATLAB:catenate:dimensionMismatch') +end + function testSubsrefWithNonScalarSubs(testCase) data = rand(100, 1); pipe = types.untyped.DataPipe('data', data); @@ -296,6 +332,54 @@ function testSubsrefWithNonScalarSubs(testCase) testCase.verifyEqual(subData, data(1:5)) end +function testOverrideBoundPipeProperties(testCase) + import matlab.unittest.fixtures.SuppressedWarningsFixture + testCase.applyFixture(SuppressedWarningsFixture('NWB:DataPipe:UnusedArguments')) + + data = rand(10, 1); + pipe = types.untyped.DataPipe('data', data); + + filename = 'testInit.h5'; + datasetName = '/test_data'; + fid = H5F.create(filename); + pipe.export(fid, datasetName, {}); + H5F.close(fid); + + loadedPipe = types.untyped.DataPipe('filename', filename, 'path', datasetName, 'dataType', 'double'); + + % Using verifyError did not work for the following statements, i.e this: + % testCase.verifyError(@(x) eval('loadedPipe.chunkSize = 2'), 'NWB:BoundPipe:CannotSetPipeProperty') %#ok + % fails with the following error: Attempt to add "loadedPipe" to a static workspace. + try + loadedPipe.chunkSize = 2; + catch ME + testCase.verifyEqual(ME.identifier, 'NWB:BoundPipe:CannotSetPipeProperty') + end + + try + loadedPipe.hasShuffle = false; + catch ME + testCase.verifyEqual(ME.identifier, 'NWB:BoundPipe:CannotSetPipeProperty') + end + +end + +function testDynamicFilterIsInDatasetCreationPropertyList(testCase) + import types.untyped.datapipe.dynamic.Filter; + import types.untyped.datapipe.properties.DynamicFilter; + + dcpl = H5P.create('H5P_DATASET_CREATE'); + dynamicFilter = DynamicFilter(Filter.LZ4); + + tf = dynamicFilter.isInDcpl(dcpl); + testCase.verifyFalse(tf) + + % Add filter + dynamicFilter.addTo(dcpl) + tf = dynamicFilter.isInDcpl(dcpl); + testCase.verifyTrue(tf) +end + function data = createData(dataType, size) data = randi(intmax(dataType), size, dataType); end diff --git a/+types/+untyped/+datapipe/+properties/DynamicFilter.m b/+types/+untyped/+datapipe/+properties/DynamicFilter.m index 4579ec276..151746443 100644 --- a/+types/+untyped/+datapipe/+properties/DynamicFilter.m +++ b/+types/+untyped/+datapipe/+properties/DynamicFilter.m @@ -10,14 +10,15 @@ methods function obj = DynamicFilter(filter, parameters) - validateattributes(filter, ... - {'types.untyped.datapipe.dynamic.Filter'}, ... - {'scalar'}, ... - 'DynamicFilter', 'filter'); + arguments + filter (1,1) types.untyped.datapipe.dynamic.Filter + parameters = [] + end + assert(~verLessThan('matlab', '9.12'), ... ['Your MATLAB version `%s` does not support writing with ' ... 'dynamically loaded filters. Please upgrade to version R2022a ' ... - 'or higher in order to use this feature.'], version); + 'or higher in order to use this feature.'], version); %#ok assert(H5Z.filter_avail(uint32(filter)), ... ['Filter `%s` does not appear to be installed on this system. ' ... 'Please doublecheck `%s` for more information about writing ' ... @@ -26,15 +27,10 @@ 'https://www.mathworks.com/help/matlab/import_export/read-and-write-hdf5-datasets-using-dynamically-loaded-filters.html'); obj.dynamicFilter = filter; - - if (1 < nargin) - obj.parameters = parameters; - else - obj.parameters = []; - end + obj.parameters = parameters; end - function tf = isInDcpl(dcpl) + function tf = isInDcpl(obj, dcpl) tf = false; for i = 0:(H5P.get_nfilters(dcpl) - 1) From d7b9eddf9ae8950f27d3aea6bdf2f928214351f0 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 10 Nov 2024 17:29:28 +0100 Subject: [PATCH 20/32] Add unittests for functions in +types namespace --- +tests/+unit/+types/FunctionTests.m | 71 +++++++++++++++++++++++++++ +types/+untyped/+datapipe/BoundPipe.m | 2 +- 2 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 +tests/+unit/+types/FunctionTests.m diff --git a/+tests/+unit/+types/FunctionTests.m b/+tests/+unit/+types/FunctionTests.m new file mode 100644 index 000000000..0f2836af5 --- /dev/null +++ b/+tests/+unit/+types/FunctionTests.m @@ -0,0 +1,71 @@ +classdef FunctionTests < matlab.unittest.TestCase +% FunctionTests - Unit test for functions in +types namespace. + + methods (Test) + function testcheckConstraint(testCase) + pname = 'vectordata'; + name = 'col1'; + namedprops = struct('col1', 'double'); + constrained = {'types.hdmf_common.VectorData'}; + val = []; + + % Should pass with no error + types.util.checkConstraint(pname, name, namedprops, constrained, val) + + val = 10; + types.util.checkConstraint(pname, name, namedprops, constrained, val) + + val = {10}; + testCase.verifyError(... + @(varargin) types.util.checkConstraint(pname, name, namedprops, constrained, val), ... + 'NWB:TypeCorrection:InvalidConversion') + + % Verify that checkConstraint fails if constrained is not a + % char describing a type (test unexpected error) + constrained = {false}; + namedprops = struct.empty; + testCase.verifyError(... + @(varargin) types.util.checkConstraint(pname, name, namedprops, constrained, val), ... + 'MATLAB:string:MustBeStringScalarOrCharacterVector') + end + + function testCheckDimsWithValidSize(testCase) + types.util.checkDims([3,5], {[3,5]}) + testCase.verifyTrue(true) + end + + function testCheckDimsWithInvalidSize(testCase) + testCase.verifyError(... + @(varargin) types.util.checkDims([3,5], {[1,10,4]}), ... + 'NWB:CheckDims:InvalidDimensions' ) + end + + function testCheckDtype(testCase) + % Example that triggers a block for non-scalar structs in + % compound data processing case. %Todo: simplify + ccss = types.core.VoltageClampStimulusSeries( ... + 'data', [1, 2, 3, 4, 5] ); + vcs = types.core.VoltageClampSeries( ... + 'data', [0.1, 0.2, 0.3, 0.4, 0.5] ); + + stimuli = types.core.IntracellularStimuliTable( ... + 'colnames', {'stimulus'}, ... + 'id', types.hdmf_common.ElementIdentifiers( ... + 'data', int64([0, 1, 2]) ... + ), ... + 'stimulus', types.core.TimeSeriesReferenceVectorData( ... + 'data', struct( ... + 'idx_start', {0, 1, -1}, ... + 'count', {5, 3, -1}, ... + 'timeseries', { ... + types.untyped.ObjectView(ccss), ... + types.untyped.ObjectView(ccss), ... + types.untyped.ObjectView(vcs) ... + } ... + )... + )... + ); + testCase.verifyClass(stimuli, 'types.core.IntracellularStimuliTable') + end + end +end \ No newline at end of file diff --git a/+types/+untyped/+datapipe/BoundPipe.m b/+types/+untyped/+datapipe/BoundPipe.m index 4539b692d..d9e63d530 100644 --- a/+types/+untyped/+datapipe/BoundPipe.m +++ b/+types/+untyped/+datapipe/BoundPipe.m @@ -242,13 +242,13 @@ function setPipeProperty(~, ~) end function tf = hasPipeProperty(obj, name) + tf = false; for i = 1:length(obj.pipeProperties) if isa(obj.pipeProperties{i}, name) tf = true; return; end end - tf = false; end function removePipeProperty(~, ~) From 7469fa522f116f706bee375cc45aeba5bd5bf352 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 10 Nov 2024 17:46:06 +0100 Subject: [PATCH 21/32] Add class setup to +types function tests --- +tests/+unit/+types/FunctionTests.m | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/+tests/+unit/+types/FunctionTests.m b/+tests/+unit/+types/FunctionTests.m index 0f2836af5..956b6a59f 100644 --- a/+tests/+unit/+types/FunctionTests.m +++ b/+tests/+unit/+types/FunctionTests.m @@ -1,6 +1,19 @@ classdef FunctionTests < matlab.unittest.TestCase % FunctionTests - Unit test for functions in +types namespace. + methods (TestClassSetup) + function setupClass(testCase) + % Get the root path of the matnwb repository + rootPath = misc.getMatnwbDir(); + % Use a fixture to add the folder to the search path + testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath)); + + % Use a fixture to create a temporary working directory + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + + generateCore('savedir', '.') + end + end methods (Test) function testcheckConstraint(testCase) pname = 'vectordata'; From 0282719fadc4dd6119eb41b82f370a960ad6083c Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 10 Nov 2024 22:25:42 +0100 Subject: [PATCH 22/32] Add tests for clearing dynamictable plus fix related bugs --- +tests/+sanity/GenerationTest.m | 2 ++ +tests/+unit/+common/ValidatorTest.m | 3 ++- +tests/+unit/dynamicTableTest.m | 33 ++++++++++++++++++++++++ +types/+util/+dynamictable/checkConfig.m | 9 +++---- +types/+util/+dynamictable/clear.m | 20 ++++++++++---- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/+tests/+sanity/GenerationTest.m b/+tests/+sanity/GenerationTest.m index 1b30f54c9..23f2d8a6f 100644 --- a/+tests/+sanity/GenerationTest.m +++ b/+tests/+sanity/GenerationTest.m @@ -7,6 +7,8 @@ function setupClass(testCase) rootPath = fullfile(fileparts(mfilename('fullpath')), '..', '..'); testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath)); + nwbClearGenerated() + testCase.addTeardown(@(varargin) generateCore('savedir','.')) end end diff --git a/+tests/+unit/+common/ValidatorTest.m b/+tests/+unit/+common/ValidatorTest.m index ab59a29c3..c4a6d9e59 100644 --- a/+tests/+unit/+common/ValidatorTest.m +++ b/+tests/+unit/+common/ValidatorTest.m @@ -2,7 +2,8 @@ % ValidatorTest - Unit test for validators. methods (Test) - function testInvalidVersionNumberFormat(testCase) testCase.verifyError( ... + function testInvalidVersionNumberFormat(testCase) + testCase.verifyError( ... @(vn) matnwb.common.mustBeValidSchemaVersion('1.0'), ... 'NWB:VersionValidator:InvalidVersionNumber') end diff --git a/+tests/+unit/dynamicTableTest.m b/+tests/+unit/dynamicTableTest.m index e82267e23..a4acdb06c 100644 --- a/+tests/+unit/dynamicTableTest.m +++ b/+tests/+unit/dynamicTableTest.m @@ -41,6 +41,39 @@ function testNwbToTableWithReferencedTablesAsTableRows(testCase) testCase.verifyClass(row1colB, 'table') end +function testClearDynamicTable(testCase) + dtr_table = createDynamicTableWithTableRegionReferences(); + types.util.dynamictable.clear(dtr_table) + + % testCase.verifyEmpty(dtr_table.vectordata) %todo when PR merged + testCase.verifyEqual(size(dtr_table.vectordata), uint64([0,1])) +end + +function testClearDynamicTableV2_1(testCase) + + import matlab.unittest.fixtures.SuppressedWarningsFixture + testCase.applyFixture(SuppressedWarningsFixture('NWB:CheckUnset:InvalidProperties')) + + nwbClearGenerated('.', 'ClearCache', true) + generateCore("2.1.0", "savedir", '.') + rehash(); + table = types.core.DynamicTable( ... + 'description', 'test table with DynamicTableRegion', ... + 'colnames', {'dtr_col_a', 'dtr_col_b'}, ... + 'dtr_col_a', 1:4, ... + 'dtr_col_b', 5:8, ... + 'id', types.core.ElementIdentifiers('data', [0; 1; 2; 3]) ); + + types.util.dynamictable.clear(table) + + % testCase.verifyEmpty(dtr_table.vectordata) %todo when PR merged + testCase.verifyEqual(size(table.vectordata), uint64([0,1])) + + nwbClearGenerated('.','ClearCache',true) + generateCore('savedir', '.'); + rehash(); +end + % Non-test functions function dtr_table = createDynamicTableWithTableRegionReferences() % Create a dynamic table with two columns, where the data of each column is diff --git a/+types/+util/+dynamictable/checkConfig.m b/+types/+util/+dynamictable/checkConfig.m index 2725c6dfa..66061a8e8 100644 --- a/+types/+util/+dynamictable/checkConfig.m +++ b/+types/+util/+dynamictable/checkConfig.m @@ -1,4 +1,4 @@ -function checkConfig(DynamicTable, varargin) +function checkConfig(DynamicTable, ignoreList) % CHECKCONFIG Given a DynamicTable object, this functions checks for proper % DynamicTable configuration % @@ -13,10 +13,9 @@ function checkConfig(DynamicTable, varargin) % 1) The length of all columns in the dynamic table is the same. % 2) All rows have a corresponding id. If none exist, this function creates them. % 3) No index loops exist. - if nargin<2 - ignoreList = {}; - else - ignoreList = varargin{1}; + arguments + DynamicTable + ignoreList (1,:) cell = {}; end if isempty(DynamicTable.colnames) diff --git a/+types/+util/+dynamictable/clear.m b/+types/+util/+dynamictable/clear.m index 83056b87f..cff609812 100644 --- a/+types/+util/+dynamictable/clear.m +++ b/+types/+util/+dynamictable/clear.m @@ -1,13 +1,23 @@ function clear(DynamicTable) %CLEAR Given a valid DynamicTable object, clears all rows and type % information in the table. -validateattributes(DynamicTable, {'types.hdmf_common.DynamicTable'}, {'scalar'}); -DynamicTable.id = types.hdmf_common.ElementIdentifiers(); +validateattributes(DynamicTable, {'types.hdmf_common.DynamicTable', 'types.core.DynamicTable'}, {'scalar'}); +if isa(DynamicTable, 'types.core.DynamicTable') % Schema version <2.2.0 + elementIdentifierClass = @types.core.ElementIdentifiers; + vectorDataClassName = 'types.core.VectorData'; + vectorIndexClassName = 'types.core.VectorIndex'; +else + elementIdentifierClass = @types.hdmf_common.ElementIdentifiers; + vectorDataClassName = 'types.hdmf_common.VectorData'; + vectorIndexClassName = 'types.hdmf_common.VectorIndex'; +end + +DynamicTable.id = elementIdentifierClass(); DynamicTable.vectordata = types.untyped.Set(@(nm, val)types.util.checkConstraint(... - 'vectordata', nm, struct(), {'types.hdmf_common.VectorData'}, val)); + 'vectordata', nm, struct(), {vectorDataClassName}, val)); if isprop(DynamicTable, 'vectorindex') % Schema version <2.3.0 DynamicTable.vectorindex = types.untyped.Set(@(nm, val)types.util.checkConstraint(... - 'vectorindex', nm, struct(), {'types.hdmf_common.VectorIndex'}, val)); + 'vectorindex', nm, struct(), {vectorIndexClassName}, val)); +end end -end \ No newline at end of file From 2f30b4d128982cd3eddb81113bfc85dec9ae321a Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sun, 10 Nov 2024 22:25:58 +0100 Subject: [PATCH 23/32] Add input options for nwbClearGenerated --- nwbClearGenerated.m | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/nwbClearGenerated.m b/nwbClearGenerated.m index 43a4bae12..8e0c6cfd0 100644 --- a/nwbClearGenerated.m +++ b/nwbClearGenerated.m @@ -1,13 +1,26 @@ -function clearedNamespaceNames = nwbClearGenerated() +function clearedNamespaceNames = nwbClearGenerated(targetFolder, options) %% NWBCLEARGENERATED clears generated class files. - nwbDir = misc.getMatnwbDir(); - typesPath = fullfile(nwbDir, '+types'); + arguments + targetFolder (1,1) string {mustBeFolder} = misc.getMatnwbDir() + options.ClearCache (1,1) logical = false + end + typesPath = fullfile(targetFolder, '+types'); listing = dir(typesPath); moduleNames = setdiff({listing.name}, {'+untyped', '+util', '.', '..'}); generatedPaths = fullfile(typesPath, moduleNames); for i=1:length(generatedPaths) rmdir(generatedPaths{i}, 's'); end + + if options.ClearCache + cachePath = fullfile(targetFolder, 'namespaces'); + listing = dir(fullfile(cachePath, '*.mat')); + generatedPaths = fullfile(cachePath, {listing.name}); + for i=1:length(generatedPaths) + delete(generatedPaths{i}); + end + end + if nargout == 1 % Return names of cleared namespaces [~, clearedNamespaceNames] = fileparts(generatedPaths); clearedNamespaceNames = strrep(clearedNamespaceNames, '+', ''); From 6fc75ce12d95c4f041cf69ffa9a9ff02b09f5de4 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 10:32:56 +0100 Subject: [PATCH 24/32] Add cleanup for writeAttribute Removes lines that can not be reached --- +io/writeAttribute.m | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/+io/writeAttribute.m b/+io/writeAttribute.m index 93f5b1ebc..f257d029b 100644 --- a/+io/writeAttribute.m +++ b/+io/writeAttribute.m @@ -3,22 +3,22 @@ function writeAttribute(fid, fullpath, data, varargin) [tid, sid, data] = io.mapData2H5(fid, data, varargin{:}); [path, name] = io.pathParts(fullpath); if isempty(path) - path = '/'; %weird case if the property is in root + path = '/'; % Weird case if the property is in root end oid = H5O.open(fid, path, 'H5P_DEFAULT'); +h5CleanupObj = onCleanup(@(sid_, oid_) closeSpaceAndObject(sid, oid) ); + try id = H5A.create(oid, name, tid, sid, 'H5P_DEFAULT'); catch ME - %when a dataset is copied over, it also copies all attributes with it. - %So we have to open the Attribute for overwriting instead. + % When a dataset is copied over, it also copies all attributes with it. + % So we have to open the Attribute for overwriting instead. % this may also happen if the attribute is a reference if contains(ME.message, 'H5A__create attribute already exists')... || contains(ME.message, 'H5A_create attribute already exists') H5A.delete(oid, name); id = H5A.create(oid, name, tid, sid, 'H5P_DEFAULT'); else - H5O.close(oid); - H5S.close(sid); rethrow(ME); end end @@ -26,6 +26,9 @@ function writeAttribute(fid, fullpath, data, varargin) H5A.write(id, tid, data); end H5A.close(id); -H5S.close(sid); -H5O.close(oid); + +function closeSpaceAndObject(spaceId, objectId) + H5S.close(spaceId); + H5O.close(objectId); +end end \ No newline at end of file From 53057caf10952dcaa79f1630edcb96f79384b3b8 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 10:33:05 +0100 Subject: [PATCH 25/32] Update Point.m Simplify --- +io/+space/+shape/Point.m | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/+io/+space/+shape/Point.m b/+io/+space/+shape/Point.m index 83d105e4f..d7f94d7ec 100644 --- a/+io/+space/+shape/Point.m +++ b/+io/+space/+shape/Point.m @@ -22,11 +22,9 @@ end function varargout = getMatlabIndex(obj) - if 0 == nargout - return; + if nargout > 0 + varargout{1} = obj.index; end - - varargout{1} = obj.index; end end end From ac3b9906c07229352bb30c23e477fc33114e0b64 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 12:26:48 +0100 Subject: [PATCH 26/32] Add SpaceTest --- +tests/+unit/+io/SpaceTest.m | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 +tests/+unit/+io/SpaceTest.m diff --git a/+tests/+unit/+io/SpaceTest.m b/+tests/+unit/+io/SpaceTest.m new file mode 100644 index 000000000..9395d7a8a --- /dev/null +++ b/+tests/+unit/+io/SpaceTest.m @@ -0,0 +1,25 @@ +classdef SpaceTest < matlab.unittest.TestCase +% SpaceTest - Unit test for io.space.* namespace. + + methods (Test) + function testEmptyInput(testCase) + shape = io.space.findShapes([]); + + testCase.verifyClass(shape, 'cell') + testCase.verifyLength(shape, 1) + testCase.verifyClass(shape{1}, 'io.space.shape.Block') + end + + function testSegmentSelection(testCase) + shape = io.space.segmentSelection({1:10}, [1,100]); + + testCase.verifyClass(shape, 'cell') + end + + function testPoint(testCase) + point = io.space.shape.Point(1); + + testCase.verifyEqual(point.getMatlabIndex, 1) + end + end +end \ No newline at end of file From 3aaed8fbdb4230ed2614da3a50d4eb1770de216e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 12:45:03 +0100 Subject: [PATCH 27/32] Fix bug with writing and parsing logical data in compound data type --- +io/parseCompound.m | 8 ++++- +io/writeCompound.m | 5 ++- +tests/+unit/+io/WriteTest.m | 70 ++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 +tests/+unit/+io/WriteTest.m diff --git a/+io/parseCompound.m b/+io/parseCompound.m index 983224a4f..35bf451c8 100644 --- a/+io/parseCompound.m +++ b/+io/parseCompound.m @@ -51,6 +51,12 @@ logicalFieldName = fieldName(isLogicalType); for iFieldName = 1:length(logicalFieldName) name = logicalFieldName{iFieldName}; - data.(name) = strcmp('TRUE', data.(name)); + if isa(data.(name), 'int8') + data.(name) = logical(data.(name)); + elseif isa(data.(name), 'cell') && ismember(string(data.(name){1}), ["TRUE", "FALSE"]) + data.(name) = strcmp('TRUE', data.(name)); + else + error('NWB:ParseCompound:UnknownLogicalFormat', 'Could not resolve data of logical type') + end end end \ No newline at end of file diff --git a/+io/writeCompound.m b/+io/writeCompound.m index 1e1b7131c..eca732dba 100644 --- a/+io/writeCompound.m +++ b/+io/writeCompound.m @@ -95,7 +95,7 @@ function writeCompound(fid, fullpath, data, varargin) % convert logical values boolNames = names(strcmp(classes, 'logical')); for iField = 1:length(boolNames) - data.(boolNames{iField}) = strcmp('TRUE', data.(boolNames{iField})); + data.(boolNames{iField}) = int8(data.(boolNames{iField})); end %transpose numeric column arrays to row arrays @@ -134,6 +134,9 @@ function writeCompound(fid, fullpath, data, varargin) warning('NWB:WriteCompund:ContinuousCompoundResize', ... 'Attempted to change size of continuous compound `%s`. Skipping.', ... fullpath); + H5D.close(did); + H5S.close(sid); + return end end H5P.close(create_plist); diff --git a/+tests/+unit/+io/WriteTest.m b/+tests/+unit/+io/WriteTest.m new file mode 100644 index 000000000..8cb7cd2fb --- /dev/null +++ b/+tests/+unit/+io/WriteTest.m @@ -0,0 +1,70 @@ +classdef WriteTest < matlab.unittest.TestCase +% WriteTest - Unit test for io.write* functions. + + methods (TestMethodSetup) + function setup(testCase) + % Use a fixture to create a temporary working directory + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + end + end + + methods (Test) + + function testWriteBooleanAttribute(testCase) + filename = 'temp_test_file.h5'; + fid = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(id) H5F.close(fid)); + + targetPath = '/'; + io.writeGroup(fid, targetPath) + + % Define target dataset path and create it in the HDF5 file + io.writeAttribute(fid, '/test', true); % First write to create the dataset + + % Read using h5readatt and confirm value + value = h5readatt(filename, '/', 'test'); + testCase.verifyTrue( strcmp(value, 'TRUE')) + + % Read using io.parseAttributes and confirm value + blackList = struct(... + 'attributes', {{'.specloc', 'object_id'}},... + 'groups', {{}}); + + S = h5info(filename); + [attributeProperties, ~] =... + io.parseAttributes(filename, S.Attributes, S.Name, blackList); + testCase.verifyTrue(attributeProperties('test')) + end + + function testWriteCompound(testCase) + % Create a temporary HDF5 file + filename = 'temp_test_file.h5'; + fullPath = '/test_dataset'; + fid = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(id) H5F.close(fid)); + + % Data to write + data = struct('a', {1,2}, 'b', {true, false}, 'c', {'test', 'new test'}); + io.writeCompound(fid, fullPath, data); % First write to create the dataset + + loadedData = h5read(filename, '/test_dataset'); + tempT = struct2table(loadedData); + % Booleans are loaded as int8, need to manually fix + tempT.b = logical( tempT.b ); + loadedData = table2struct(tempT)'; + testCase.verifyEqual(data, loadedData); + + % Use parse compound + did = H5D.open(fid, '/test_dataset'); + fsid = H5D.get_space(did); + loadedData = H5D.read(did, 'H5ML_DEFAULT', fsid, fsid,... + 'H5P_DEFAULT'); + data = io.parseCompound(did, loadedData); + H5S.close(fsid); + H5D.close(did); + + parsedData = table2struct( struct2table(loadedData) )'; + testCase.verifyEqual(data, parsedData); + end + end +end \ No newline at end of file From 0749a6aada8d8b98becacac080469f1d1b3011bb Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 12:46:38 +0100 Subject: [PATCH 28/32] Fix variableName in test --- +tests/+unit/+io/WriteTest.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/+tests/+unit/+io/WriteTest.m b/+tests/+unit/+io/WriteTest.m index 8cb7cd2fb..83b023fe2 100644 --- a/+tests/+unit/+io/WriteTest.m +++ b/+tests/+unit/+io/WriteTest.m @@ -59,11 +59,11 @@ function testWriteCompound(testCase) fsid = H5D.get_space(did); loadedData = H5D.read(did, 'H5ML_DEFAULT', fsid, fsid,... 'H5P_DEFAULT'); - data = io.parseCompound(did, loadedData); + parsedData = io.parseCompound(did, loadedData); H5S.close(fsid); H5D.close(did); - parsedData = table2struct( struct2table(loadedData) )'; + parsedData = table2struct( struct2table(parsedData) )'; testCase.verifyEqual(data, parsedData); end end From 2b12acbce64d392a0a6893b253daf5af0a8db55f Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 13:16:59 +0100 Subject: [PATCH 29/32] Add more unittests to WriteTest --- +tests/+unit/+io/WriteTest.m | 73 ++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/+tests/+unit/+io/WriteTest.m b/+tests/+unit/+io/WriteTest.m index 83b023fe2..ead733b09 100644 --- a/+tests/+unit/+io/WriteTest.m +++ b/+tests/+unit/+io/WriteTest.m @@ -36,6 +36,26 @@ function testWriteBooleanAttribute(testCase) testCase.verifyTrue(attributeProperties('test')) end + function testWriteDatasetOverwrite(testCase) + + % Create a temporary HDF5 file + filename = 'temp_test_file.h5'; + fullPath = '/test_dataset'; + fid = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(id) H5F.close(fid)); + + % Initial data to write (e.g., 10x10) + initialData = rand(10, 10); + io.writeDataset(fid, fullPath, initialData); % First write to create the dataset + + % Attempt to write data of a different size (e.g., 5x5) + newData = rand(5, 5); + testCase.verifyWarning(... + @(varargin) io.writeDataset(fid, fullPath, newData), ... + 'NWB:WriteDataset:ContinuousDatasetResize' ... + ) + end + function testWriteCompound(testCase) % Create a temporary HDF5 file filename = 'temp_test_file.h5'; @@ -66,5 +86,58 @@ function testWriteCompound(testCase) parsedData = table2struct( struct2table(parsedData) )'; testCase.verifyEqual(data, parsedData); end + + function testWriteCompoundOverWrite(testCase) + + % Create a temporary HDF5 file + filename = 'temp_test_file.h5'; + fullPath = '/test_dataset'; + fid = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(id) H5F.close(fid)); + + % Initial data to write (e.g., 10x10) + initialData = struct('a', 1, 'b', true, 'c', 'test'); + io.writeCompound(fid, fullPath, initialData); % First write to create the dataset + + % Attempt to write data of a different size (e.g., 5x5) + newData = cat(1, initialData, struct('a', 2, 'b', false, 'c', 'new test')); + testCase.verifyWarning(... + @(varargin) io.writeCompound(fid, fullPath, newData), ... + 'NWB:WriteCompund:ContinuousCompoundResize' ... + ) + end + + function testWriteGroupWithPathThatEndsWithSlash(testCase) + filename = 'temp_test_file.h5'; + fullPath = '/test_group/'; + fid = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(id) H5F.close(fid)); + groupExists = io.writeGroup(fid, fullPath); + testCase.verifyFalse(groupExists) + + S = h5info(filename); + testCase.verifyEqual(S.Groups.Name, '/test_group') + end + + function testWriteSoftLink(testCase) + % Create a temporary HDF5 file + filename = 'temp_test_file.h5'; + fid = H5F.create(filename, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(id) H5F.close(fid)); + + % Define target dataset path and create it in the HDF5 file + targetPath = '/dataset'; + initialData = rand(10, 10); + io.writeDataset(fid, targetPath, initialData); % First write to create the dataset + + % Define soft link name and use writeSoftLink to create it + linkName = 'soft_link_to_dataset'; + io.writeSoftLink(targetPath, fid, linkName); + + S = h5info(filename); + testCase.verifyTrue(strcmp(S.Links.Name, linkName)) + testCase.verifyTrue(strcmp(S.Links.Type, 'soft link')) + testCase.verifyTrue(strcmp(S.Links.Value{1}, targetPath)) + end end end \ No newline at end of file From befd8fea9c93db737043379bee422bb5ae2b7a5b Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 14:37:26 +0100 Subject: [PATCH 30/32] Remove unused function --- +types/+util/checkDependent.m | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 +types/+util/checkDependent.m diff --git a/+types/+util/checkDependent.m b/+types/+util/checkDependent.m deleted file mode 100644 index f1b723443..000000000 --- a/+types/+util/checkDependent.m +++ /dev/null @@ -1,11 +0,0 @@ -function checkDependent(parent, children, unconstructed) - if ~any(strcmp(parent, unconstructed)) - for i=1:length(children) - child = children{i}; - if any(strcmp(child, unconstructed)) - error('NWB:CheckDependentType:TypeRequiredForParent', ... - 'Dependent type `%s` is required for parent property `%s`', child, parent); - end - end - end -end \ No newline at end of file From a6c61d4476bd82755b8d5c7481fe990ac83e0bf5 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 14:38:06 +0100 Subject: [PATCH 31/32] Add unit tests for functions in +types namespace --- +tests/+unit/+types/FunctionTests.m | 45 ++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/+tests/+unit/+types/FunctionTests.m b/+tests/+unit/+types/FunctionTests.m index 956b6a59f..2adce77aa 100644 --- a/+tests/+unit/+types/FunctionTests.m +++ b/+tests/+unit/+types/FunctionTests.m @@ -80,5 +80,48 @@ function testCheckDtype(testCase) ); testCase.verifyClass(stimuli, 'types.core.IntracellularStimuliTable') end - end + + function testParseConstrainedAppendMode(testCase) + + columnA = types.hdmf_common.VectorData( ... + 'description', 'first column', ... + 'data', rand(10,1) ... + ); + + % 1D column + idCol = types.hdmf_common.ElementIdentifiers('data', int64(0:9)'); + + % Create table + dynamicTable = types.hdmf_common.DynamicTable(... + 'description', 'test dynamic table column',... + 'colnames', {'colA'}, ... + 'colA', columnA, ... + 'id', idCol ... + ); + + columnB = types.hdmf_common.VectorData( ... + 'description', 'second column', ... + 'data', rand(10,1) ... + ); + + + [vectordata, ~] = types.util.parseConstrained(dynamicTable, ... + 'vectordata', 'types.hdmf_common.VectorData', ... + 'colB', columnB ); + + testCase.verifyEqual(vectordata.keys, {'colA', 'colB'}) + testCase.verifyEqual(vectordata.get('colA').data, columnA.data) + testCase.verifyEqual(vectordata.get('colB').data, columnB.data) + end + + function testCorrectType(testCase) + testCase.verifyEqual(types.util.correctType('5', 'double'), 5) + testCase.verifyEqual(types.util.correctType(uint8(5), 'int32'), int32(5)) + testCase.verifyEqual(types.util.correctType(uint32(5), 'int32'), int64(5)) + + testCase.verifyWarning(... + @(varargin) types.util.correctType('5i', 'double'), ... + 'NWB:TypeCorrection:DataLoss') + end + end end \ No newline at end of file From 00d16235255bf1ae66d7e8faf1877cab3e49cf17 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 11 Nov 2024 17:23:00 +0100 Subject: [PATCH 32/32] Change exist to isfolder/isfile --- +spec/saveCache.m | 2 +- +tests/+system/PyNWBIOTest.m | 2 +- +tests/+util/getPythonPath.m | 2 +- +types/+untyped/ExternalLink.m | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/+spec/saveCache.m b/+spec/saveCache.m index 17d10b6ea..9ed160742 100644 --- a/+spec/saveCache.m +++ b/+spec/saveCache.m @@ -2,7 +2,7 @@ function saveCache(NamespaceInfo, saveDir) %SAVECACHE saves namespace info as .mat in `namespaces` directory namespacePath = fullfile(saveDir, 'namespaces'); -if 7 ~= exist(namespacePath, 'dir') +if ~isfolder(namespacePath) mkdir(namespacePath); end diff --git a/+tests/+system/PyNWBIOTest.m b/+tests/+system/PyNWBIOTest.m index d08c2053c..792177288 100644 --- a/+tests/+system/PyNWBIOTest.m +++ b/+tests/+system/PyNWBIOTest.m @@ -37,7 +37,7 @@ function testInFromPyNWB(testCase) setenv('PYTHONPATH', fileparts(mfilename('fullpath'))); envPath = fullfile('+tests', 'env.mat'); - if 2 == exist(envPath, 'file') + if isfile(envPath) Env = load(envPath, '-mat'); if isfield(Env, 'pythonPath') pythonPath = Env.pythonPath; diff --git a/+tests/+util/getPythonPath.m b/+tests/+util/getPythonPath.m index 3a43dc0ac..b7b3c494f 100644 --- a/+tests/+util/getPythonPath.m +++ b/+tests/+util/getPythonPath.m @@ -1,7 +1,7 @@ function pythonPath = getPythonPath() envPath = fullfile('+tests', 'env.mat'); - if 2 == exist(envPath, 'file') + if isfile(envPath) Env = load(envPath, '-mat'); if isfield(Env, 'pythonPath') pythonPath = Env.pythonPath; diff --git a/+types/+untyped/ExternalLink.m b/+types/+untyped/ExternalLink.m index e5d566adf..23f0af166 100644 --- a/+types/+untyped/ExternalLink.m +++ b/+types/+untyped/ExternalLink.m @@ -28,7 +28,7 @@ % if path is valid hdf5 path, then returns either a Nwb Object, DataStub, or Link Object % otherwise errors, probably. assert(ischar(Link.filename), 'expecting filename to be a char array.'); - assert(2 == exist(Link.filename, 'file'), '%s does not exist.', Link.filename); + assert(isfile(Link.filename), '%s does not exist.', Link.filename); fid = H5F.open(Link.filename, 'H5F_ACC_RDONLY', 'H5P_DEFAULT'); LinkedInfo = h5info(Link.filename, Link.path);