From a458d62fe65cc467a35770713b5513f98db407f9 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 4 Nov 2024 20:09:42 +0100 Subject: [PATCH 01/47] Only include namespaces for types that are included in NWB file on export (Issue #607) --- NwbFile.m | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/NwbFile.m b/NwbFile.m index cc88b1ddb..8efd8c4c6 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -101,6 +101,20 @@ function export(obj, filename) typename,... varargin{:}); end + + function nwbTypeNames = listNwbTypes(obj) + % listNwbTypes - List all unique NWB types in file + objectMap = searchProperties(containers.Map, obj, '', ''); + + objects = objectMap.values(); + objectClassNames = cellfun(@(c) string(class(c)), objects); + objectClassNames = unique(objectClassNames); + + keep = startsWith(objectClassNames, "types."); + ignore = startsWith(objectClassNames, "types.untyped"); + + nwbTypeNames = objectClassNames(keep & ~ignore); + end end %% PRIVATE @@ -131,7 +145,7 @@ function resolveReferences(obj, fid, references) end end - function embedSpecifications(~, fid) + function embedSpecifications(obj, fid) try attrId = H5A.open(fid, '/.specloc'); specLocation = H5R.get_name(fid, 'H5R_OBJECT', H5A.read(attrId)); @@ -144,6 +158,15 @@ function embedSpecifications(~, fid) end JsonData = schemes.exportJson(); + + % Only embed namespaces for types that are included in the file + includedNwbTypes = obj.listNwbTypes(); + namespaceNames = getNamespacesOfTypes(includedNwbTypes); + + allMatlabNamespaceNames = strrep({JsonData.name}, '-', '_'); + [~, keepIdx] = intersect(allMatlabNamespaceNames, namespaceNames, 'stable'); + JsonData = JsonData(keepIdx); + for iJson = 1:length(JsonData) JsonDatum = JsonData(iJson); schemaNamespaceLocation = strjoin({specLocation, JsonDatum.name}, '/'); @@ -244,4 +267,19 @@ function embedSpecifications(~, fid) searchProperties(pathToObjectMap, propValue, fullPath, typename, varargin{:}); end end -end \ No newline at end of file +end + +function namespaceNames = getNamespacesOfTypes(nwbTypeNames) +% getNamespacesOfTypes - Get namespace names for a list of nwb types + arguments + nwbTypeNames (1,:) string + end + + namespaceNames = repmat("", size(nwbTypeNames)); + pattern = '[types.]+\.(\w+)\.'; + + for i = 1:numel(nwbTypeNames) + namespaceNames(i) = regexp(nwbTypeNames(i), pattern, 'tokens', 'once'); + end + namespaceNames = unique(namespaceNames); +end From 35150e9bb1d6796fb6d017ecec43d90c2677b2fa Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 20:14:07 +0100 Subject: [PATCH 02/47] Add functionality for installing extensions --- +matnwb/+extension/dispExtensionInfo.m | 15 +++ +matnwb/+extension/installAll.m | 6 + +matnwb/+extension/installExtension.m | 126 ++++++++++++++++++ +matnwb/+extension/listExtensions.m | 53 ++++++++ .gitignore | 11 ++ nwbInstallExtension.m | 79 +++++++++++ .../nwbInstallExtension.txt | 35 +++++ .../matnwb_createNwbInstallExtension.m | 28 ++++ 8 files changed, 353 insertions(+) create mode 100644 +matnwb/+extension/dispExtensionInfo.m create mode 100644 +matnwb/+extension/installAll.m create mode 100644 +matnwb/+extension/installExtension.m create mode 100644 +matnwb/+extension/listExtensions.m create mode 100644 nwbInstallExtension.m create mode 100644 resources/function_templates/nwbInstallExtension.txt create mode 100644 tools/maintenance/matnwb_createNwbInstallExtension.m diff --git a/+matnwb/+extension/dispExtensionInfo.m b/+matnwb/+extension/dispExtensionInfo.m new file mode 100644 index 000000000..337c35235 --- /dev/null +++ b/+matnwb/+extension/dispExtensionInfo.m @@ -0,0 +1,15 @@ +function dispExtensionInfo(extensionName) + arguments + extensionName (1,1) string + end + + T = matnwb.extension.listExtensions(); + isMatch = T.name == extensionName; + extensionList = join( compose(" %s", [T.name]), newline ); + assert( ... + any(isMatch), ... + 'NWB:DisplayExtensionMetadata:ExtensionNotFound', ... + 'Extension "%s" was not found in the extension catalog:\n%s', extensionName, extensionList) + metadata = table2struct(T(isMatch, :)); + disp(metadata) +end diff --git a/+matnwb/+extension/installAll.m b/+matnwb/+extension/installAll.m new file mode 100644 index 000000000..a4fff669c --- /dev/null +++ b/+matnwb/+extension/installAll.m @@ -0,0 +1,6 @@ +function installAll() + T = matnwb.extension.listExtensions(); + for i = 1:height(T) + matnwb.extension.installExtension( T.name(i) ) + end +end diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m new file mode 100644 index 000000000..48f30044c --- /dev/null +++ b/+matnwb/+extension/installExtension.m @@ -0,0 +1,126 @@ +function installExtension(extensionName) + arguments + extensionName (1,1) string + end + + repoTargetFolder = fullfile(userpath, "NWB-Extension-Source"); + if ~isfolder(repoTargetFolder); mkdir(repoTargetFolder); end + + T = matnwb.extension.listExtensions(); + isMatch = T.name == extensionName; + + extensionList = join( compose(" %s", [T.name]), newline ); + assert( ... + any(isMatch), ... + 'NWB:InstallExtension:ExtensionNotFound', ... + 'Extension "%s" was not found in the extension catalog:\n', extensionList) + + defaultBranchNames = ["main", "master"]; + + wasDownloaded = false; + for i = 1:2 + try + repositoryUrl = T{isMatch, 'src'}; + if endsWith(repositoryUrl, '/') + repositoryUrl = extractBefore(repositoryUrl, strlength(repositoryUrl)); + end + if contains(repositoryUrl, 'github.com') + downloadUrl = sprintf( '%s/archive/refs/heads/%s.zip', repositoryUrl, defaultBranchNames(i) ); + + elseif contains(repositoryUrl, 'gitlab.com') + repoPathSegments = strsplit(repositoryUrl, '/'); + repoName = repoPathSegments{end}; + downloadUrl = sprintf( '%s/-/archive/%s/%s-%s.zip', ... + repositoryUrl, defaultBranchNames(i), repoName, defaultBranchNames(i)); + else + error('NWB:InstallExtension:UnknownRepository', ... + 'Extension "%s" is located in an unsupported repository / source location. Please create an issue on matnwb''s github page', extensionName) + end + repoTargetFolder = downloadZippedRepo(downloadUrl, repoTargetFolder, true, true); + wasDownloaded = true; + break + catch ME + if strcmp(ME.identifier, 'MATLAB:webservices:HTTP404StatusCodeError') + continue + else + rethrow(ME) + end + end + end + if ~wasDownloaded + error('NWB:InstallExtension:DownloadFailed', ... + 'Failed to download spec for extension "%s"', extensionName) + end + L = dir(fullfile(repoTargetFolder, 'spec', '*namespace.yaml')); + assert(... + ~isempty(L), ... + 'NWB:InstallExtension:NamespaceNotFound', ... + 'No namespace file was found for extension "%s"', extension.Identifier ... + ) + assert(... + numel(L)==1, ... + 'NWB:InstallExtension:MultipleNamespacesFound', ... + 'More than one namespace file was found for extension "%s"', extension.Identifier ... + ) + generateExtension( fullfile(L.folder, L.name) ); +end + +function repoFolder = downloadZippedRepo(githubUrl, targetFolder, updateFlag, throwErrorIfFails) +%downloadZippedRepo Download zipped repo + + if nargin < 3; updateFlag = false; end + if nargin < 4; throwErrorIfFails = false; end + + if isa(updateFlag, 'char') && strcmp(updateFlag, 'update') + updateFlag = true; + end + + % Create a temporary path for storing the downloaded file. + [~, ~, fileType] = fileparts(githubUrl); + tempFilepath = [tempname, fileType]; + + % Download the file containing the addon toolbox + try + tempFilepath = websave(tempFilepath, githubUrl); + fileCleanupObj = onCleanup( @(fname) delete(tempFilepath) ); + catch ME + if throwErrorIfFails + rethrow(ME) + end + end + + unzippedFiles = unzip(tempFilepath, tempdir); + unzippedFolder = unzippedFiles{1}; + if endsWith(unzippedFolder, filesep) + unzippedFolder = unzippedFolder(1:end-1); + end + + [~, repoFolderName] = fileparts(unzippedFolder); + targetFolder = fullfile(targetFolder, repoFolderName); + + if updateFlag && isfolder(targetFolder) + + % Delete current version + if isfolder(targetFolder) + if contains(path, fullfile(targetFolder, filesep)) + pathList = strsplit(path, pathsep); + pathList_ = pathList(startsWith(pathList, fullfile(targetFolder, filesep))); + rmpath(strjoin(pathList_, pathsep)) + end + try + rmdir(targetFolder, 's') + catch + warning('Could not remove old installation... Please report') + end + end + else + % pass + end + + movefile(unzippedFolder, targetFolder); + + % Delete the temp zip file + clear fileCleanupObj + + repoFolder = targetFolder; +end diff --git a/+matnwb/+extension/listExtensions.m b/+matnwb/+extension/listExtensions.m new file mode 100644 index 000000000..e8ae59c1b --- /dev/null +++ b/+matnwb/+extension/listExtensions.m @@ -0,0 +1,53 @@ +function extensionTable = listExtensions(options) + arguments + options.Refresh (1,1) logical = false + end + + persistent extensionRecords + + if isempty(extensionRecords) || options.Refresh + catalogUrl = "https://raw.githubusercontent.com/nwb-extensions/nwb-extensions.github.io/refs/heads/main/data/records.json"; + extensionRecords = jsondecode(webread(catalogUrl)); + extensionRecords = consolidateStruct(extensionRecords); + + extensionRecords = struct2table(extensionRecords); + + fieldsKeep = ["name", "version", "last_updated", "src", "license", "maintainers", "readme"]; + extensionRecords = extensionRecords(:, fieldsKeep); + + for name = fieldsKeep + if ischar(extensionRecords.(name){1}) + extensionRecords.(name) = string(extensionRecords.(name)); + end + end + end + extensionTable = extensionRecords; +end + +function structArray = consolidateStruct(S) + % Get all field names of S + mainFields = fieldnames(S); + + % Initialize an empty struct array + structArray = struct(); + + % Iterate over each field of S + for i = 1:numel(mainFields) + subStruct = S.(mainFields{i}); % Extract sub-struct + + % Add all fields of the sub-struct to the struct array + fields = fieldnames(subStruct); + for j = 1:numel(fields) + structArray(i).(fields{j}) = subStruct.(fields{j}); + end + end + + % Ensure consistency by filling missing fields with [] + allFields = unique([fieldnames(structArray)]); + for i = 1:numel(structArray) + missingFields = setdiff(allFields, fieldnames(structArray(i))); + for j = 1:numel(missingFields) + structArray(i).(missingFields{j}) = []; + end + end +end diff --git a/.gitignore b/.gitignore index 988501fa9..c4dc14548 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,14 @@ workspace/ *.swp .DS_Store +tests/env.mat + +# Ignore everything in the +types/ folder ++types/* + +# Explicitly include these subdirectories +!+types/+core/ +!+types/+hdmf_common/ +!+types/+hdmf_experimental/ +!+types/+untyped/ +!+types/+util/ + diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m new file mode 100644 index 000000000..1c7d85161 --- /dev/null +++ b/nwbInstallExtension.m @@ -0,0 +1,79 @@ +function nwbInstallExtension(extensionName) +% nwbInstallExtension - Installs a specified NWB extension. +% +% nwbInstallExtension(extensionName) installs a Neurodata Without Borders +% (NWB) extension to extend the functionality of the core NWB schemas. +% extensionName is a scalar string or a string array, containing the name +% of one or more extensions from the Neurodata Extesions Catalog +% +% Usage: +% nwbInstallExtension(extensionName) +% +% Valid Extension Names: +% - "ndx-miniscope" +% - "ndx-simulation-output" +% - "ndx-ecog" +% - "ndx-fret" +% - "ndx-icephys-meta" +% - "ndx-events" +% - "ndx-nirs" +% - "ndx-hierarchical-behavioral-data" +% - "ndx-sound" +% - "ndx-extract" +% - "ndx-photometry" +% - "ndx-acquisition-module" +% - "ndx-odor-metadata" +% - "ndx-whisk" +% - "ndx-ecg" +% - "ndx-franklab-novela" +% - "ndx-photostim" +% - "ndx-multichannel-volume" +% - "ndx-depth-moseq" +% - "ndx-probeinterface" +% - "ndx-dbs" +% - "ndx-hed" +% - "ndx-ophys-devices" +% +% Example: +% % Install the "ndx-miniscope" extension +% nwbInstallExtension("ndx-miniscope") +% +% See also: +% matnwb.extension.listExtensions, matnwb.extension.installExtension + + arguments + extensionName (1,:) string {mustBeMember(extensionName, [... + "ndx-miniscope", ... + "ndx-simulation-output", ... + "ndx-ecog", ... + "ndx-fret", ... + "ndx-icephys-meta", ... + "ndx-events", ... + "ndx-nirs", ... + "ndx-hierarchical-behavioral-data", ... + "ndx-sound", ... + "ndx-extract", ... + "ndx-photometry", ... + "ndx-acquisition-module", ... + "ndx-odor-metadata", ... + "ndx-whisk", ... + "ndx-ecg", ... + "ndx-franklab-novela", ... + "ndx-photostim", ... + "ndx-multichannel-volume", ... + "ndx-depth-moseq", ... + "ndx-probeinterface", ... + "ndx-dbs", ... + "ndx-hed", ... + "ndx-ophys-devices" ... + ] ... + )} = [] + end + if isempty(extensionName) + T = matnwb.extension.listExtensions(); + extensionList = join( compose(" %s", [T.name]), newline ); + error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) + else + matnwb.extension.installExtension(extensionName) + end +end diff --git a/resources/function_templates/nwbInstallExtension.txt b/resources/function_templates/nwbInstallExtension.txt new file mode 100644 index 000000000..2a7f22a8a --- /dev/null +++ b/resources/function_templates/nwbInstallExtension.txt @@ -0,0 +1,35 @@ +function nwbInstallExtension(extensionName) +% nwbInstallExtension - Installs a specified NWB extension. +% +% nwbInstallExtension(extensionName) installs a Neurodata Without Borders +% (NWB) extension to extend the functionality of the core NWB schemas. +% extensionName is a scalar string or a string array, containing the name +% of one or more extensions from the Neurodata Extesions Catalog +% +% Usage: +% nwbInstallExtension(extensionName) +% +% Valid Extension Names: +{{extensionNamesDoc}} +% +% Example: +% % Install the "ndx-miniscope" extension +% nwbInstallExtension("ndx-miniscope") +% +% See also: +% matnwb.extension.listExtensions, matnwb.extension.installExtension + + arguments + extensionName (1,:) string {mustBeMember(extensionName, [... +{{extensionNames}} ... + ] ... + )} = [] + end + if isempty(extensionName) + T = matnwb.extension.listExtensions(); + extensionList = join( compose(" %s", [T.name]), newline ); + error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) + else + matnwb.extension.installExtension(extensionName) + end +end diff --git a/tools/maintenance/matnwb_createNwbInstallExtension.m b/tools/maintenance/matnwb_createNwbInstallExtension.m new file mode 100644 index 000000000..f9af731dc --- /dev/null +++ b/tools/maintenance/matnwb_createNwbInstallExtension.m @@ -0,0 +1,28 @@ +function matnwb_createNwbInstallExtension() +% matnwb_createNwbInstallExtension - Create nwbInstallExtension from template +% +% This function can be run to update the list of available extension +% names in the function's arguments block based on the neurodata +% extensions catalog + + matnwbRootDir = misc.getMatnwbDir(); + fcnTemplate = fileread(fullfile(matnwbRootDir, ... + 'resources', 'function_templates', 'nwbInstallExtension.txt')); + + extensionTable = matnwb.extension.listExtensions(); + extensionNames = extensionTable.name; + + indentStr = repmat(' ', 1, 12); + extensionNamesStr = compose("%s""%s""", indentStr, extensionNames); + extensionNamesStr = strjoin(extensionNamesStr, ", ..." + newline); + fcnStr = replace(fcnTemplate, "{{extensionNames}}", extensionNamesStr); + + extensionNamesStr = compose("%% - ""%s""", extensionNames); + extensionNamesStr = strjoin(extensionNamesStr, newline); + fcnStr = replace(fcnStr, "{{extensionNamesDoc}}", extensionNamesStr); + + + fid = fopen(fullfile(matnwbRootDir, 'nwbInstallExtension.m'), "wt"); + fwrite(fid, fcnStr); + fclose(fid); +end From bd408943f8ba1985494b234984d0f5184d8932ff Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 20:27:41 +0100 Subject: [PATCH 03/47] Minor fixes --- +matnwb/+extension/installExtension.m | 10 ++++++++-- nwbInstallExtension.m | 20 +++++++++---------- .../nwbInstallExtension.txt | 20 +++++++++---------- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m index 48f30044c..e349e6733 100644 --- a/+matnwb/+extension/installExtension.m +++ b/+matnwb/+extension/installExtension.m @@ -1,4 +1,10 @@ function installExtension(extensionName) +% installExtension - Install NWB extension from Neurodata Extensions Catalog +% +% matnwb.extension.nwbInstallExtension(extensionNames) installs Neurodata +% Without Borders (NWB) extensions to extend the functionality of the core +% NWB schemas. + arguments extensionName (1,1) string end @@ -55,12 +61,12 @@ function installExtension(extensionName) assert(... ~isempty(L), ... 'NWB:InstallExtension:NamespaceNotFound', ... - 'No namespace file was found for extension "%s"', extension.Identifier ... + 'No namespace file was found for extension "%s"', extensionName ... ) assert(... numel(L)==1, ... 'NWB:InstallExtension:MultipleNamespacesFound', ... - 'More than one namespace file was found for extension "%s"', extension.Identifier ... + 'More than one namespace file was found for extension "%s"', extensionName ... ) generateExtension( fullfile(L.folder, L.name) ); end diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index 1c7d85161..61bf0f6a4 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -1,13 +1,11 @@ -function nwbInstallExtension(extensionName) +function nwbInstallExtension(extensionNames) % nwbInstallExtension - Installs a specified NWB extension. % -% nwbInstallExtension(extensionName) installs a Neurodata Without Borders -% (NWB) extension to extend the functionality of the core NWB schemas. -% extensionName is a scalar string or a string array, containing the name -% of one or more extensions from the Neurodata Extesions Catalog -% % Usage: -% nwbInstallExtension(extensionName) +% nwbInstallExtension(extensionNames) installs Neurodata Without Borders +% (NWB) extensions to extend the functionality of the core NWB schemas. +% extensionNames is a scalar string or a string array, containing the name +% of one or more extensions from the Neurodata Extensions Catalog % % Valid Extension Names: % - "ndx-miniscope" @@ -42,7 +40,7 @@ function nwbInstallExtension(extensionName) % matnwb.extension.listExtensions, matnwb.extension.installExtension arguments - extensionName (1,:) string {mustBeMember(extensionName, [... + extensionNames (1,:) string {mustBeMember(extensionNames, [... "ndx-miniscope", ... "ndx-simulation-output", ... "ndx-ecog", ... @@ -69,11 +67,13 @@ function nwbInstallExtension(extensionName) ] ... )} = [] end - if isempty(extensionName) + if isempty(extensionNames) T = matnwb.extension.listExtensions(); extensionList = join( compose(" %s", [T.name]), newline ); error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) else - matnwb.extension.installExtension(extensionName) + for extensionName = extensionNames + matnwb.extension.installExtension(extensionName) + end end end diff --git a/resources/function_templates/nwbInstallExtension.txt b/resources/function_templates/nwbInstallExtension.txt index 2a7f22a8a..14f090da0 100644 --- a/resources/function_templates/nwbInstallExtension.txt +++ b/resources/function_templates/nwbInstallExtension.txt @@ -1,13 +1,11 @@ -function nwbInstallExtension(extensionName) +function nwbInstallExtension(extensionNames) % nwbInstallExtension - Installs a specified NWB extension. % -% nwbInstallExtension(extensionName) installs a Neurodata Without Borders -% (NWB) extension to extend the functionality of the core NWB schemas. -% extensionName is a scalar string or a string array, containing the name -% of one or more extensions from the Neurodata Extesions Catalog -% % Usage: -% nwbInstallExtension(extensionName) +% nwbInstallExtension(extensionNames) installs Neurodata Without Borders +% (NWB) extensions to extend the functionality of the core NWB schemas. +% extensionNames is a scalar string or a string array, containing the name +% of one or more extensions from the Neurodata Extensions Catalog % % Valid Extension Names: {{extensionNamesDoc}} @@ -20,16 +18,18 @@ function nwbInstallExtension(extensionName) % matnwb.extension.listExtensions, matnwb.extension.installExtension arguments - extensionName (1,:) string {mustBeMember(extensionName, [... + extensionNames (1,:) string {mustBeMember(extensionNames, [... {{extensionNames}} ... ] ... )} = [] end - if isempty(extensionName) + if isempty(extensionNames) T = matnwb.extension.listExtensions(); extensionList = join( compose(" %s", [T.name]), newline ); error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) else - matnwb.extension.installExtension(extensionName) + for extensionName = extensionNames + matnwb.extension.installExtension(extensionName) + end end end From 132aa67fa18828f96dcbc9f6bbbd1793a447aa9e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 20:32:41 +0100 Subject: [PATCH 04/47] Update comment --- +matnwb/+extension/installExtension.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m index e349e6733..9cd79a31b 100644 --- a/+matnwb/+extension/installExtension.m +++ b/+matnwb/+extension/installExtension.m @@ -85,7 +85,7 @@ function installExtension(extensionName) [~, ~, fileType] = fileparts(githubUrl); tempFilepath = [tempname, fileType]; - % Download the file containing the addon toolbox + % Download the file containing the zipped repository try tempFilepath = websave(tempFilepath, githubUrl); fileCleanupObj = onCleanup( @(fname) delete(tempFilepath) ); From 781f303bdff045e63409311fd105914f65ecafe1 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 20:38:43 +0100 Subject: [PATCH 05/47] Add comment + print message when extension has been installed --- +matnwb/+extension/installExtension.m | 1 + nwbInstallExtension.m | 2 +- resources/function_templates/nwbInstallExtension.txt | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m index 9cd79a31b..a86a34f4d 100644 --- a/+matnwb/+extension/installExtension.m +++ b/+matnwb/+extension/installExtension.m @@ -69,6 +69,7 @@ function installExtension(extensionName) 'More than one namespace file was found for extension "%s"', extensionName ... ) generateExtension( fullfile(L.folder, L.name) ); + fprintf("Installed extension ""%s"".\n", extensionName) end function repoFolder = downloadZippedRepo(githubUrl, targetFolder, updateFlag, throwErrorIfFails) diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index 61bf0f6a4..5052fc32c 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -7,7 +7,7 @@ function nwbInstallExtension(extensionNames) % extensionNames is a scalar string or a string array, containing the name % of one or more extensions from the Neurodata Extensions Catalog % -% Valid Extension Names: +% Valid Extension Names (from https://nwb-extensions.github.io): % - "ndx-miniscope" % - "ndx-simulation-output" % - "ndx-ecog" diff --git a/resources/function_templates/nwbInstallExtension.txt b/resources/function_templates/nwbInstallExtension.txt index 14f090da0..bd8226f0c 100644 --- a/resources/function_templates/nwbInstallExtension.txt +++ b/resources/function_templates/nwbInstallExtension.txt @@ -7,7 +7,7 @@ function nwbInstallExtension(extensionNames) % extensionNames is a scalar string or a string array, containing the name % of one or more extensions from the Neurodata Extensions Catalog % -% Valid Extension Names: +% Valid Extension Names (from https://nwb-extensions.github.io): {{extensionNamesDoc}} % % Example: From eab998e5206996e57899e87272087a60c33fd676 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 20:43:08 +0100 Subject: [PATCH 06/47] Update installExtension.m --- +matnwb/+extension/installExtension.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m index a86a34f4d..9ec3c8678 100644 --- a/+matnwb/+extension/installExtension.m +++ b/+matnwb/+extension/installExtension.m @@ -1,9 +1,9 @@ function installExtension(extensionName) % installExtension - Install NWB extension from Neurodata Extensions Catalog % -% matnwb.extension.nwbInstallExtension(extensionNames) installs Neurodata -% Without Borders (NWB) extensions to extend the functionality of the core -% NWB schemas. +% matnwb.extension.nwbInstallExtension(extensionName) installs a Neurodata +% Without Borders (NWB) extension from the Neurodata Extensions Catalog to +% extend the functionality of the core NWB schemas. arguments extensionName (1,1) string From be4be3d96e6871a338c9577adb60dcd10cc9c795 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 20:50:45 +0100 Subject: [PATCH 07/47] Fix changed variable name --- NwbFile.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NwbFile.m b/NwbFile.m index 45a90c2a4..d806417ee 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -75,7 +75,7 @@ function export(obj, filename, mode) includedNwbTypes = obj.listNwbTypes(); namespaceNames = getNamespacesOfTypes(includedNwbTypes); - allMatlabNamespaceNames = strrep({JsonData.name}, '-', '_'); + allMatlabNamespaceNames = strrep({jsonSpecs.name}, '-', '_'); [~, keepIdx] = intersect(allMatlabNamespaceNames, namespaceNames, 'stable'); jsonSpecs = jsonSpecs(keepIdx); From 650ceec5897d79b3c470bd4f65d4ecb5e439d0b9 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Sat, 23 Nov 2024 21:00:12 +0100 Subject: [PATCH 08/47] Update matnwb_createNwbInstallExtension.m Update docstring --- tools/maintenance/matnwb_createNwbInstallExtension.m | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/maintenance/matnwb_createNwbInstallExtension.m b/tools/maintenance/matnwb_createNwbInstallExtension.m index f9af731dc..b056d8bdc 100644 --- a/tools/maintenance/matnwb_createNwbInstallExtension.m +++ b/tools/maintenance/matnwb_createNwbInstallExtension.m @@ -1,8 +1,10 @@ function matnwb_createNwbInstallExtension() % matnwb_createNwbInstallExtension - Create nwbInstallExtension from template % -% This function can be run to update the list of available extension -% names in the function's arguments block based on the neurodata +% Running this function will update the nwbInstallExtension function in +% the root directory of the matnwb package. It will update the list of +% available extension names in the nwbInstallExtension function's arguments +% block and docstring based on the available records in the neurodata % extensions catalog matnwbRootDir = misc.getMatnwbDir(); From 8e18448b30638bf81336b1883bd2abff3b557b39 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 25 Nov 2024 11:17:16 +0100 Subject: [PATCH 09/47] Create listNwbTypeHierarchy.m Add utility function for listing the type hierarchy of an nwb type --- +schemes/+utility/listNwbTypeHierarchy.m | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 +schemes/+utility/listNwbTypeHierarchy.m diff --git a/+schemes/+utility/listNwbTypeHierarchy.m b/+schemes/+utility/listNwbTypeHierarchy.m new file mode 100644 index 000000000..432c9c804 --- /dev/null +++ b/+schemes/+utility/listNwbTypeHierarchy.m @@ -0,0 +1,21 @@ +function parentTypeNames = listNwbTypeHierarchy(nwbTypeName) +% listNwbTypeHierarchy - List the NWB type hierarchy for an NWB type + arguments + nwbTypeName (1,1) string + end + + parentTypeNames = string.empty; % Initialize an empty cell array + currentType = nwbTypeName; % Start with the specific type + + while ~strcmp(currentType, 'types.untyped.MetaClass') + parentTypeNames(end+1) = currentType; %#ok + + % Use MetaClass information to get the parent type + metaClass = meta.class.fromName(currentType); + if isempty(metaClass.SuperclassList) + break; % Reached the base type + end + % NWB parent type should always be the first superclass in the list + currentType = metaClass.SuperclassList(1).Name; + end +end From 7c41ca64a42be9f679c2fe25f97d3597db3cb96b Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 25 Nov 2024 11:18:09 +0100 Subject: [PATCH 10/47] Add private method for embedding specifications to file on export --- NwbFile.m | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/NwbFile.m b/NwbFile.m index d806417ee..7274f65de 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -69,17 +69,7 @@ function export(obj, filename, mode) end try - jsonSpecs = schemes.exportJson(); - - % Only embed namespaces for types that are included in the file - includedNwbTypes = obj.listNwbTypes(); - namespaceNames = getNamespacesOfTypes(includedNwbTypes); - - allMatlabNamespaceNames = strrep({jsonSpecs.name}, '-', '_'); - [~, keepIdx] = intersect(allMatlabNamespaceNames, namespaceNames, 'stable'); - jsonSpecs = jsonSpecs(keepIdx); - - io.spec.writeEmbeddedSpecifications(output_file_id, jsonSpecs); + obj.embedSpecifications(output_file_id) refs = export@types.core.NWBFile(obj, output_file_id, '/', {}); obj.resolveReferences(output_file_id, refs); H5F.close(output_file_id); @@ -136,6 +126,31 @@ function export(obj, filename, mode) %% PRIVATE methods(Access=private) + function embedSpecifications(obj, output_file_id) + jsonSpecs = schemes.exportJson(); + + % Resolve the name of all types and parent types that are + % included in this file, in order to only include the specs for + % namespaces of types that are included in the file. + includedNwbTypes = obj.listNwbTypes(); + includedNwbTypesWithParents = string.empty; + for i = 1:numel(includedNwbTypes) + typeHierarchy = schemes.utility.listNwbTypeHierarchy(includedNwbTypes{i}); + includedNwbTypesWithParents = [includedNwbTypesWithParents, typeHierarchy]; %#ok + end + + % Get the namespace names + namespaceNames = getNamespacesOfTypes(includedNwbTypes); + + % In the specs, the hyphen (-) is used as a word separator, while in + % matnwb the underscore (_) is used. Translate names here: + allMatlabNamespaceNames = strrep({jsonSpecs.name}, '-', '_'); + [~, keepIdx] = intersect(allMatlabNamespaceNames, namespaceNames, 'stable'); + jsonSpecs = jsonSpecs(keepIdx); + + io.spec.writeEmbeddedSpecifications(output_file_id, jsonSpecs); + end + function resolveReferences(obj, fid, references) while ~isempty(references) resolved = false(size(references)); From a467c46a04ffcf2f97fd09ea8fba111d8ee427fe Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 25 Nov 2024 11:20:15 +0100 Subject: [PATCH 11/47] Fix variable name --- NwbFile.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NwbFile.m b/NwbFile.m index 7274f65de..62b273a1c 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -140,7 +140,7 @@ function embedSpecifications(obj, output_file_id) end % Get the namespace names - namespaceNames = getNamespacesOfTypes(includedNwbTypes); + namespaceNames = getNamespacesOfTypes(includedNwbTypesWithParents); % In the specs, the hyphen (-) is used as a word separator, while in % matnwb the underscore (_) is used. Translate names here: From ff95e98c806dce82f8699e689b100e7e23f33beb Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 28 Nov 2024 13:00:17 +0100 Subject: [PATCH 12/47] Add workflow for updating nwbInstallExtension --- .github/workflows/update_extension_list.yml | 44 +++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 .github/workflows/update_extension_list.yml diff --git a/.github/workflows/update_extension_list.yml b/.github/workflows/update_extension_list.yml new file mode 100644 index 000000000..d5f3b6ed6 --- /dev/null +++ b/.github/workflows/update_extension_list.yml @@ -0,0 +1,44 @@ +name: Update extension list + +on: + workflow_dispatch: + +permissions: + contents: write + +jobs: + update_extension_list: + runs-on: ubuntu-latest + steps: + # Use deploy key to push back to protected branch + - name: Checkout repository using deploy key + uses: actions/checkout@v4 + with: + ref: refs/heads/main + ssh-key: ${{ secrets.DEPLOY_KEY }} + + - name: Install MATLAB + uses: matlab-actions/setup-matlab@v2 + + - name: Update extension list in nwbInstallExtensions + uses: matlab-actions/run-command@v2 + with: + command: | + addpath(genpath("tools")); + matnwb_createNwbInstallExtension(); + + - name: Commit the updated nwbInstallExtension function + run: | + set -e # Exit script on error + git config user.name "${{ github.workflow }} by ${{ github.actor }}" + git config user.email "<>" + git pull --rebase # Ensure the branch is up-to-date + + if [[ -n $(git status --porcelain nwbInstallExtension.m) ]]; then + git add nwbInstallExtension.m + git commit -m "Update list of extensions in nwbInstallExtension" + git push + else + echo "Nothing to commit" + fi + \ No newline at end of file From bb3514b965ed2982066c0ec573b575863fdd6637 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 28 Nov 2024 13:16:59 +0100 Subject: [PATCH 13/47] Add option to save extension in custom location --- +matnwb/+extension/installExtension.m | 5 +++-- nwbInstallExtension.m | 5 +++-- resources/function_templates/nwbInstallExtension.txt | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m index 9ec3c8678..c3590a4dc 100644 --- a/+matnwb/+extension/installExtension.m +++ b/+matnwb/+extension/installExtension.m @@ -1,4 +1,4 @@ -function installExtension(extensionName) +function installExtension(extensionName, options) % installExtension - Install NWB extension from Neurodata Extensions Catalog % % matnwb.extension.nwbInstallExtension(extensionName) installs a Neurodata @@ -7,6 +7,7 @@ function installExtension(extensionName) arguments extensionName (1,1) string + options.savedir (1,1) string = misc.getMatnwbDir() end repoTargetFolder = fullfile(userpath, "NWB-Extension-Source"); @@ -68,7 +69,7 @@ function installExtension(extensionName) 'NWB:InstallExtension:MultipleNamespacesFound', ... 'More than one namespace file was found for extension "%s"', extensionName ... ) - generateExtension( fullfile(L.folder, L.name) ); + generateExtension( fullfile(L.folder, L.name), 'savedir', options.savedir ); fprintf("Installed extension ""%s"".\n", extensionName) end diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index 5052fc32c..bbb0f672b 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -1,4 +1,4 @@ -function nwbInstallExtension(extensionNames) +function nwbInstallExtension(extensionNames, options) % nwbInstallExtension - Installs a specified NWB extension. % % Usage: @@ -66,6 +66,7 @@ function nwbInstallExtension(extensionNames) "ndx-ophys-devices" ... ] ... )} = [] + options.savedir (1,1) string = misc.getMatnwbDir() end if isempty(extensionNames) T = matnwb.extension.listExtensions(); @@ -73,7 +74,7 @@ function nwbInstallExtension(extensionNames) error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) else for extensionName = extensionNames - matnwb.extension.installExtension(extensionName) + matnwb.extension.installExtension(extensionName, 'savedir', options.savedir) end end end diff --git a/resources/function_templates/nwbInstallExtension.txt b/resources/function_templates/nwbInstallExtension.txt index bd8226f0c..1500b7118 100644 --- a/resources/function_templates/nwbInstallExtension.txt +++ b/resources/function_templates/nwbInstallExtension.txt @@ -1,4 +1,4 @@ -function nwbInstallExtension(extensionNames) +function nwbInstallExtension(extensionNames, options) % nwbInstallExtension - Installs a specified NWB extension. % % Usage: @@ -22,6 +22,7 @@ function nwbInstallExtension(extensionNames) {{extensionNames}} ... ] ... )} = [] + options.savedir (1,1) string = misc.getMatnwbDir() end if isempty(extensionNames) T = matnwb.extension.listExtensions(); @@ -29,7 +30,7 @@ function nwbInstallExtension(extensionNames) error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) else for extensionName = extensionNames - matnwb.extension.installExtension(extensionName) + matnwb.extension.installExtension(extensionName, 'savedir', options.savedir) end end end From 184fa8109be2350dd380a325f62839ff65663681 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 28 Nov 2024 13:17:04 +0100 Subject: [PATCH 14/47] Create InstallExtensionTest.m --- +tests/+unit/InstallExtensionTest.m | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 +tests/+unit/InstallExtensionTest.m diff --git a/+tests/+unit/InstallExtensionTest.m b/+tests/+unit/InstallExtensionTest.m new file mode 100644 index 000000000..afce4c567 --- /dev/null +++ b/+tests/+unit/InstallExtensionTest.m @@ -0,0 +1,49 @@ +classdef InstallExtensionTest < 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); + generateCore('savedir', '.'); + end + end + + methods (Test) + function testInstallExtension(testCase) + nwbInstallExtension("ndx-miniscope", 'savedir', '.') + + testCase.verifyTrue(isfolder('./+types/+ndx_miniscope'), ... + 'Folder with extension types does not exist') + end + + function testUseInstalledExtension(testCase) + nwbObject = testCase.initNwbFile(); + + miniscopeDevice = types.ndx_miniscope.Miniscope(... + 'deviceType', 'test_device', ... + 'compression', 'GREY', ... + 'frameRate', '30fps', ... + 'framesPerFile', int8(100) ); + + nwbObject.general_devices.set('TestMiniscope', miniscopeDevice); + + testCase.verifyClass(nwbObject.general_devices.get('TestMiniscope'), ... + 'types.ndx_miniscope.Miniscope') + end + end + + methods (Static) + function nwb = initNwbFile() + nwb = NwbFile( ... + 'session_description', 'test file for nwb extension', ... + 'identifier', 'export_test', ... + 'session_start_time', datetime("now", 'TimeZone', 'local') ); + end + end +end From ddbe9dc9550f3fe2bf0ae6ccec1816532b2e5e4c Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 6 Dec 2024 12:38:08 +0100 Subject: [PATCH 15/47] Update docstring --- nwbInstallExtension.m | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index bbb0f672b..65268f8e1 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -32,9 +32,10 @@ function nwbInstallExtension(extensionNames, options) % - "ndx-hed" % - "ndx-ophys-devices" % -% Example: -% % Install the "ndx-miniscope" extension -% nwbInstallExtension("ndx-miniscope") +% Usage: +% Example 1 - Install "ndx-miniscope" extension:: +% +% nwbInstallExtension("ndx-miniscope") % % See also: % matnwb.extension.listExtensions, matnwb.extension.installExtension From da00cea4d593d6d3b82cef625a26b9bd6a9a4489 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 12 Dec 2024 11:21:00 +0100 Subject: [PATCH 16/47] Change dispExtensionInfo to return info instead of displaying + add test --- .../+extension/{dispExtensionInfo.m => getExtensionInfo.m} | 5 +++-- +tests/+unit/InstallExtensionTest.m | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) rename +matnwb/+extension/{dispExtensionInfo.m => getExtensionInfo.m} (80%) diff --git a/+matnwb/+extension/dispExtensionInfo.m b/+matnwb/+extension/getExtensionInfo.m similarity index 80% rename from +matnwb/+extension/dispExtensionInfo.m rename to +matnwb/+extension/getExtensionInfo.m index 337c35235..411978b0c 100644 --- a/+matnwb/+extension/dispExtensionInfo.m +++ b/+matnwb/+extension/getExtensionInfo.m @@ -1,4 +1,6 @@ -function dispExtensionInfo(extensionName) +function metadata = getExtensionInfo(extensionName) +% getExtensionInfo - Get metadata for specified extension + arguments extensionName (1,1) string end @@ -11,5 +13,4 @@ function dispExtensionInfo(extensionName) 'NWB:DisplayExtensionMetadata:ExtensionNotFound', ... 'Extension "%s" was not found in the extension catalog:\n%s', extensionName, extensionList) metadata = table2struct(T(isMatch, :)); - disp(metadata) end diff --git a/+tests/+unit/InstallExtensionTest.m b/+tests/+unit/InstallExtensionTest.m index afce4c567..a2012e8b5 100644 --- a/+tests/+unit/InstallExtensionTest.m +++ b/+tests/+unit/InstallExtensionTest.m @@ -36,6 +36,13 @@ function testUseInstalledExtension(testCase) testCase.verifyClass(nwbObject.general_devices.get('TestMiniscope'), ... 'types.ndx_miniscope.Miniscope') end + + function testDisplayExtensionMetadata(testCase) + extensionName = "ndx-miniscope"; + metadata = matnwb.extension.getExtensionInfo(extensionName); + testCase.verifyClass(metadata, 'struct') + testCase.verifyEqual(metadata.name, extensionName) + end end methods (Static) From 40b7703add199aa6c823dc8e1478ae0184d5e7c6 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 12 Dec 2024 12:15:22 +0100 Subject: [PATCH 17/47] Reorganize code into separate functions and add tests --- .../+internal/downloadExtensionRepository.m | 68 +++++++++++++ .../+extension/+internal/downloadZippedRepo.m | 42 ++++++++ +matnwb/+extension/installExtension.m | 99 ++----------------- +tests/+unit/InstallExtensionTest.m | 13 +++ nwbInstallExtension.m | 4 +- 5 files changed, 133 insertions(+), 93 deletions(-) create mode 100644 +matnwb/+extension/+internal/downloadExtensionRepository.m create mode 100644 +matnwb/+extension/+internal/downloadZippedRepo.m diff --git a/+matnwb/+extension/+internal/downloadExtensionRepository.m b/+matnwb/+extension/+internal/downloadExtensionRepository.m new file mode 100644 index 000000000..fae2dee56 --- /dev/null +++ b/+matnwb/+extension/+internal/downloadExtensionRepository.m @@ -0,0 +1,68 @@ +function [wasDownloaded, repoTargetFolder] = downloadExtensionRepository(... + repositoryUrl, repoTargetFolder, extensionName) +% downloadExtensionRepository - Download the repository (source) for an extension +% +% The metadata for a neurodata extension only provides the url to the +% repository containing the extension, not the full download url. This +% function tries to download a zipped version of the repository from +% either the "main" or the "master" branch. +% +% Works for repositories located on GitHub or GitLab +% +% As of Dec. 2024, this approach works for all registered extensions + + arguments + repositoryUrl (1,1) string + repoTargetFolder (1,1) string + extensionName (1,1) string + end + + import matnwb.extension.internal.downloadZippedRepo + + defaultBranchNames = ["main", "master"]; + + wasDownloaded = false; + for i = 1:2 + try + branchName = defaultBranchNames(i); + downloadUrl = buildRepoDownloadUrl(repositoryUrl, branchName, extensionName); + repoTargetFolder = downloadZippedRepo(downloadUrl, repoTargetFolder); + wasDownloaded = true; + break + catch ME + if strcmp(ME.identifier, 'MATLAB:webservices:HTTP404StatusCodeError') + continue + else + rethrow(ME) + end + end + end +end + +function downloadUrl = buildRepoDownloadUrl(repositoryUrl, branchName, extensionName) +% buildRepoDownloadUrl - Build a download URL for a given repository and branch + arguments + repositoryUrl (1,1) string + branchName (1,1) string + extensionName (1,1) string + end + + if endsWith(repositoryUrl, '/') + repositoryUrl = extractBefore(repositoryUrl, strlength(repositoryUrl)); + end + if contains(repositoryUrl, 'github.com') + downloadUrl = sprintf( '%s/archive/refs/heads/%s.zip', repositoryUrl, branchName ); + + elseif contains(repositoryUrl, 'gitlab.com') + repoPathSegments = strsplit(repositoryUrl, '/'); + repoName = repoPathSegments{end}; + downloadUrl = sprintf( '%s/-/archive/%s/%s-%s.zip', ... + repositoryUrl, branchName, repoName, branchName); + + else + error('NWB:InstallExtension:UnknownRepository', ... + ['Extension "%s" is located in an unsupported repository ', ... + '/ source location. \nPlease create an issue on MatNWB''s ', ... + 'github page'], extensionName) + end +end diff --git a/+matnwb/+extension/+internal/downloadZippedRepo.m b/+matnwb/+extension/+internal/downloadZippedRepo.m new file mode 100644 index 000000000..6f2b2aa4d --- /dev/null +++ b/+matnwb/+extension/+internal/downloadZippedRepo.m @@ -0,0 +1,42 @@ +function repoFolder = downloadZippedRepo(githubUrl, targetFolder) +%downloadZippedRepo - Download a zipped repository + + % Create a temporary path for storing the downloaded file. + [~, ~, fileType] = fileparts(githubUrl); + tempFilepath = [tempname, fileType]; + + % Download the file containing the zipped repository + tempFilepath = websave(tempFilepath, githubUrl); + fileCleanupObj = onCleanup( @(fname) delete(tempFilepath) ); + + unzippedFiles = unzip(tempFilepath, tempdir); + unzippedFolder = unzippedFiles{1}; + if endsWith(unzippedFolder, filesep) + unzippedFolder = unzippedFolder(1:end-1); + end + + [~, repoFolderName] = fileparts(unzippedFolder); + targetFolder = fullfile(targetFolder, repoFolderName); + + if isfolder(targetFolder) + if contains(path, fullfile(targetFolder, filesep)) + pathList = strsplit(path, pathsep); + pathList_ = pathList(startsWith(pathList, fullfile(targetFolder, filesep))); + rmpath(strjoin(pathList_, pathsep)) + end + try + rmdir(targetFolder, 's') + catch + error('Could not delete previously downloaded extension which is located here:\n"%s"', targetFolder) + end + else + % pass + end + + movefile(unzippedFolder, targetFolder); + + % Delete the temp zip file + clear fileCleanupObj + + repoFolder = targetFolder; +end diff --git a/+matnwb/+extension/installExtension.m b/+matnwb/+extension/installExtension.m index c3590a4dc..b333f9d70 100644 --- a/+matnwb/+extension/installExtension.m +++ b/+matnwb/+extension/installExtension.m @@ -10,6 +10,8 @@ function installExtension(extensionName, options) options.savedir (1,1) string = misc.getMatnwbDir() end + import matnwb.extension.internal.downloadExtensionRepository + repoTargetFolder = fullfile(userpath, "NWB-Extension-Source"); if ~isfolder(repoTargetFolder); mkdir(repoTargetFolder); end @@ -22,38 +24,11 @@ function installExtension(extensionName, options) 'NWB:InstallExtension:ExtensionNotFound', ... 'Extension "%s" was not found in the extension catalog:\n', extensionList) - defaultBranchNames = ["main", "master"]; - - wasDownloaded = false; - for i = 1:2 - try - repositoryUrl = T{isMatch, 'src'}; - if endsWith(repositoryUrl, '/') - repositoryUrl = extractBefore(repositoryUrl, strlength(repositoryUrl)); - end - if contains(repositoryUrl, 'github.com') - downloadUrl = sprintf( '%s/archive/refs/heads/%s.zip', repositoryUrl, defaultBranchNames(i) ); - - elseif contains(repositoryUrl, 'gitlab.com') - repoPathSegments = strsplit(repositoryUrl, '/'); - repoName = repoPathSegments{end}; - downloadUrl = sprintf( '%s/-/archive/%s/%s-%s.zip', ... - repositoryUrl, defaultBranchNames(i), repoName, defaultBranchNames(i)); - else - error('NWB:InstallExtension:UnknownRepository', ... - 'Extension "%s" is located in an unsupported repository / source location. Please create an issue on matnwb''s github page', extensionName) - end - repoTargetFolder = downloadZippedRepo(downloadUrl, repoTargetFolder, true, true); - wasDownloaded = true; - break - catch ME - if strcmp(ME.identifier, 'MATLAB:webservices:HTTP404StatusCodeError') - continue - else - rethrow(ME) - end - end - end + repositoryUrl = T{isMatch, 'src'}; + + [wasDownloaded, repoTargetFolder] = ... + downloadExtensionRepository(repositoryUrl, repoTargetFolder, extensionName); + if ~wasDownloaded error('NWB:InstallExtension:DownloadFailed', ... 'Failed to download spec for extension "%s"', extensionName) @@ -72,63 +47,3 @@ function installExtension(extensionName, options) generateExtension( fullfile(L.folder, L.name), 'savedir', options.savedir ); fprintf("Installed extension ""%s"".\n", extensionName) end - -function repoFolder = downloadZippedRepo(githubUrl, targetFolder, updateFlag, throwErrorIfFails) -%downloadZippedRepo Download zipped repo - - if nargin < 3; updateFlag = false; end - if nargin < 4; throwErrorIfFails = false; end - - if isa(updateFlag, 'char') && strcmp(updateFlag, 'update') - updateFlag = true; - end - - % Create a temporary path for storing the downloaded file. - [~, ~, fileType] = fileparts(githubUrl); - tempFilepath = [tempname, fileType]; - - % Download the file containing the zipped repository - try - tempFilepath = websave(tempFilepath, githubUrl); - fileCleanupObj = onCleanup( @(fname) delete(tempFilepath) ); - catch ME - if throwErrorIfFails - rethrow(ME) - end - end - - unzippedFiles = unzip(tempFilepath, tempdir); - unzippedFolder = unzippedFiles{1}; - if endsWith(unzippedFolder, filesep) - unzippedFolder = unzippedFolder(1:end-1); - end - - [~, repoFolderName] = fileparts(unzippedFolder); - targetFolder = fullfile(targetFolder, repoFolderName); - - if updateFlag && isfolder(targetFolder) - - % Delete current version - if isfolder(targetFolder) - if contains(path, fullfile(targetFolder, filesep)) - pathList = strsplit(path, pathsep); - pathList_ = pathList(startsWith(pathList, fullfile(targetFolder, filesep))); - rmpath(strjoin(pathList_, pathsep)) - end - try - rmdir(targetFolder, 's') - catch - warning('Could not remove old installation... Please report') - end - end - else - % pass - end - - movefile(unzippedFolder, targetFolder); - - % Delete the temp zip file - clear fileCleanupObj - - repoFolder = targetFolder; -end diff --git a/+tests/+unit/InstallExtensionTest.m b/+tests/+unit/InstallExtensionTest.m index a2012e8b5..0cf80e68f 100644 --- a/+tests/+unit/InstallExtensionTest.m +++ b/+tests/+unit/InstallExtensionTest.m @@ -15,6 +15,12 @@ function setupClass(testCase) end methods (Test) + function testInstallExtensionFailsWithNoInputArgument(testCase) + testCase.verifyError(... + @(varargin) nwbInstallExtension(), ... + 'NWB:InstallExtension:MissingArgument') + end + function testInstallExtension(testCase) nwbInstallExtension("ndx-miniscope", 'savedir', '.') @@ -43,6 +49,13 @@ function testDisplayExtensionMetadata(testCase) testCase.verifyClass(metadata, 'struct') testCase.verifyEqual(metadata.name, extensionName) end + + function testDownloadUnknownRepository(testCase) + repositoryUrl = "https://www.unknown-repo.com/anon/my_nwb_extension"; + testCase.verifyError(... + @() matnwb.extension.internal.downloadExtensionRepository(repositoryUrl, "", "my_nwb_extension"), ... + 'NWB:InstallExtension:UnknownRepository'); + end end methods (Static) diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index 65268f8e1..1a9449cea 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -72,10 +72,12 @@ function nwbInstallExtension(extensionNames, options) if isempty(extensionNames) T = matnwb.extension.listExtensions(); extensionList = join( compose(" %s", [T.name]), newline ); - error("Please specify the name of an extension. Available extensions:\n\n%s\n", extensionList) + error('NWB:InstallExtension:MissingArgument', ... + 'Please specify the name of an extension. Available extensions:\n\n%s\n', extensionList) else for extensionName = extensionNames matnwb.extension.installExtension(extensionName, 'savedir', options.savedir) end end end + From 6faba210f6ecbe10d2a20dd62427849995ab23b9 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 12 Dec 2024 14:10:30 +0100 Subject: [PATCH 18/47] Minor changes to improve test coverage --- .../+internal/buildRepoDownloadUrl.m | 25 +++++++++++++++++++ .../+internal/downloadExtensionRepository.m | 10 ++++++-- .../+extension/+internal/downloadZippedRepo.m | 7 +----- +tests/+unit/InstallExtensionTest.m | 24 +++++++++++++++--- nwbtest.m | 5 +++- 5 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 +matnwb/+extension/+internal/buildRepoDownloadUrl.m diff --git a/+matnwb/+extension/+internal/buildRepoDownloadUrl.m b/+matnwb/+extension/+internal/buildRepoDownloadUrl.m new file mode 100644 index 000000000..59751f784 --- /dev/null +++ b/+matnwb/+extension/+internal/buildRepoDownloadUrl.m @@ -0,0 +1,25 @@ +function downloadUrl = buildRepoDownloadUrl(repositoryUrl, branchName) +% buildRepoDownloadUrl - Build a download URL for a given repository and branch + arguments + repositoryUrl (1,1) string + branchName (1,1) string + end + + if endsWith(repositoryUrl, '/') + repositoryUrl = extractBefore(repositoryUrl, strlength(repositoryUrl)); + end + + if contains(repositoryUrl, 'github.com') + downloadUrl = sprintf( '%s/archive/refs/heads/%s.zip', repositoryUrl, branchName ); + + elseif contains(repositoryUrl, 'gitlab.com') + repoPathSegments = strsplit(repositoryUrl, '/'); + repoName = repoPathSegments{end}; + downloadUrl = sprintf( '%s/-/archive/%s/%s-%s.zip', ... + repositoryUrl, branchName, repoName, branchName); + + else + error('NWB:BuildRepoDownloadUrl:UnsupportedRepository', ... + 'Expected repository URL to point to a GitHub or a GitLab repository') + end +end diff --git a/+matnwb/+extension/+internal/downloadExtensionRepository.m b/+matnwb/+extension/+internal/downloadExtensionRepository.m index fae2dee56..43d264021 100644 --- a/+matnwb/+extension/+internal/downloadExtensionRepository.m +++ b/+matnwb/+extension/+internal/downloadExtensionRepository.m @@ -18,20 +18,26 @@ end import matnwb.extension.internal.downloadZippedRepo - + import matnwb.extension.internal.buildRepoDownloadUrl + defaultBranchNames = ["main", "master"]; wasDownloaded = false; for i = 1:2 try branchName = defaultBranchNames(i); - downloadUrl = buildRepoDownloadUrl(repositoryUrl, branchName, extensionName); + downloadUrl = buildRepoDownloadUrl(repositoryUrl, branchName); repoTargetFolder = downloadZippedRepo(downloadUrl, repoTargetFolder); wasDownloaded = true; break catch ME if strcmp(ME.identifier, 'MATLAB:webservices:HTTP404StatusCodeError') continue + elseif strcmp(ME.identifier, 'NWB:BuildRepoDownloadUrl:UnsupportedRepository') + error('NWB:InstallExtension:UnsupportedRepository', ... + ['Extension "%s" is located in an unsupported repository ', ... + '/ source location. \nPlease create an issue on MatNWB''s ', ... + 'github page'], extensionName) else rethrow(ME) end diff --git a/+matnwb/+extension/+internal/downloadZippedRepo.m b/+matnwb/+extension/+internal/downloadZippedRepo.m index 6f2b2aa4d..1d3acd944 100644 --- a/+matnwb/+extension/+internal/downloadZippedRepo.m +++ b/+matnwb/+extension/+internal/downloadZippedRepo.m @@ -19,15 +19,10 @@ targetFolder = fullfile(targetFolder, repoFolderName); if isfolder(targetFolder) - if contains(path, fullfile(targetFolder, filesep)) - pathList = strsplit(path, pathsep); - pathList_ = pathList(startsWith(pathList, fullfile(targetFolder, filesep))); - rmpath(strjoin(pathList_, pathsep)) - end try rmdir(targetFolder, 's') catch - error('Could not delete previously downloaded extension which is located here:\n"%s"', targetFolder) + error('Could not delete previously downloaded extension which is located at:\n"%s"', targetFolder) end else % pass diff --git a/+tests/+unit/InstallExtensionTest.m b/+tests/+unit/InstallExtensionTest.m index 0cf80e68f..45016b88f 100644 --- a/+tests/+unit/InstallExtensionTest.m +++ b/+tests/+unit/InstallExtensionTest.m @@ -30,7 +30,7 @@ function testInstallExtension(testCase) function testUseInstalledExtension(testCase) nwbObject = testCase.initNwbFile(); - + miniscopeDevice = types.ndx_miniscope.Miniscope(... 'deviceType', 'test_device', ... 'compression', 'GREY', ... @@ -43,7 +43,7 @@ function testUseInstalledExtension(testCase) 'types.ndx_miniscope.Miniscope') end - function testDisplayExtensionMetadata(testCase) + function testGetExtensionMetadata(testCase) extensionName = "ndx-miniscope"; metadata = matnwb.extension.getExtensionInfo(extensionName); testCase.verifyClass(metadata, 'struct') @@ -54,7 +54,25 @@ function testDownloadUnknownRepository(testCase) repositoryUrl = "https://www.unknown-repo.com/anon/my_nwb_extension"; testCase.verifyError(... @() matnwb.extension.internal.downloadExtensionRepository(repositoryUrl, "", "my_nwb_extension"), ... - 'NWB:InstallExtension:UnknownRepository'); + 'NWB:InstallExtension:UnsupportedRepository'); + end + + function testBuildRepoDownloadUrl(testCase) + + import matnwb.extension.internal.buildRepoDownloadUrl + + repoUrl = buildRepoDownloadUrl('https://github.com/user/test', 'main'); + testCase.verifyEqual(repoUrl, 'https://github.com/user/test/archive/refs/heads/main.zip') + + repoUrl = buildRepoDownloadUrl('https://github.com/user/test/', 'main'); + testCase.verifyEqual(repoUrl, 'https://github.com/user/test/archive/refs/heads/main.zip') + + repoUrl = buildRepoDownloadUrl('https://gitlab.com/user/test', 'main'); + testCase.verifyEqual(repoUrl, 'https://gitlab.com/user/test/-/archive/main/test-main.zip') + + testCase.verifyError(... + @() buildRepoDownloadUrl('https://unsupported.com/user/test', 'main'), ... + 'NWB:BuildRepoDownloadUrl:UnsupportedRepository') end end diff --git a/nwbtest.m b/nwbtest.m index a1f2ed15b..c15c4ec0a 100644 --- a/nwbtest.m +++ b/nwbtest.m @@ -64,7 +64,10 @@ [installDir, ~, ~] = fileparts(mfilename('fullpath')); ignoreFolders = {'tutorials', 'tools', '+contrib', '+util', 'external_packages', '+tests'}; - ignorePaths = {fullfile('+misc', 'generateDocs.m'), [mfilename '.m'], 'nwbClearGenerated.m'}; + ignorePaths = {... + fullfile('+matnwb', '+extension', 'installAll.m'), ... + [mfilename '.m'], ... + 'nwbClearGenerated.m'}; mfilePaths = getMfilePaths(installDir, ignoreFolders, ignorePaths); if ~verLessThan('matlab', '9.3') && ~isempty(mfilePaths) runner.addPlugin(CodeCoveragePlugin.forFile(mfilePaths,... From a74a2d2227d329b6f0bc2359874ade719b58f489 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 12 Dec 2024 14:16:09 +0100 Subject: [PATCH 19/47] add nwbInstallExtension to docs --- docs/source/pages/functions/index.rst | 1 + docs/source/pages/functions/nwbInstallExtension.rst | 5 +++++ tools/documentation/private/generateRstForNwbFunctions.m | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 docs/source/pages/functions/nwbInstallExtension.rst diff --git a/docs/source/pages/functions/index.rst b/docs/source/pages/functions/index.rst index 354979185..1dd788fee 100644 --- a/docs/source/pages/functions/index.rst +++ b/docs/source/pages/functions/index.rst @@ -13,3 +13,4 @@ These are the main functions of the MatNWB API generateCore generateExtension nwbClearGenerated + nwbInstallExtension diff --git a/docs/source/pages/functions/nwbInstallExtension.rst b/docs/source/pages/functions/nwbInstallExtension.rst new file mode 100644 index 000000000..58b37cfe2 --- /dev/null +++ b/docs/source/pages/functions/nwbInstallExtension.rst @@ -0,0 +1,5 @@ +nwbInstallExtension +=================== + +.. mat:module:: . +.. autofunction:: nwbInstallExtension diff --git a/tools/documentation/private/generateRstForNwbFunctions.m b/tools/documentation/private/generateRstForNwbFunctions.m index d6f1dd680..f0a1bfce8 100644 --- a/tools/documentation/private/generateRstForNwbFunctions.m +++ b/tools/documentation/private/generateRstForNwbFunctions.m @@ -5,7 +5,7 @@ function generateRstForNwbFunctions() rootDir = misc.getMatnwbDir(); rootFiles = dir(rootDir); rootFileNames = {rootFiles.name}; - rootWhitelist = {'nwbRead.m', 'NwbFile.m', 'nwbExport.m', 'generateCore.m', 'generateExtension.m', 'nwbClearGenerated.m'};%, 'nwbInstallExtension.m'}; + rootWhitelist = {'nwbRead.m', 'NwbFile.m', 'nwbExport.m', 'generateCore.m', 'generateExtension.m', 'nwbClearGenerated.m', 'nwbInstallExtension.m'}; isWhitelisted = ismember(rootFileNames, rootWhitelist); rootFiles(~isWhitelisted) = []; From 16877f9624ee76e30b935fcfa99e2e7b3a45f29e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 12 Dec 2024 14:29:31 +0100 Subject: [PATCH 20/47] Update update_extension_list.yml Add schedule event for workflow to update nwbInstallExtension --- .github/workflows/update_extension_list.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/update_extension_list.yml b/.github/workflows/update_extension_list.yml index d5f3b6ed6..f9dfb7639 100644 --- a/.github/workflows/update_extension_list.yml +++ b/.github/workflows/update_extension_list.yml @@ -1,6 +1,11 @@ name: Update extension list on: + schedule: + # Run at 8:15 on working days [Minute Hour Day Month Weekdays] + # Run this 15 minutes after source repo is updated + # https://github.com/nwb-extensions/nwb-extensions.github.io/blob/main/.github/workflows/data.yml + - cron: 15 8 * * 0-5 workflow_dispatch: permissions: From 0bc735fc0eef0b858d739a5234790b4a900073e1 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 12 Dec 2024 14:36:10 +0100 Subject: [PATCH 21/47] Update downloadExtensionRepository.m Remove local function --- .../+internal/downloadExtensionRepository.m | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/+matnwb/+extension/+internal/downloadExtensionRepository.m b/+matnwb/+extension/+internal/downloadExtensionRepository.m index 43d264021..4c47dd18d 100644 --- a/+matnwb/+extension/+internal/downloadExtensionRepository.m +++ b/+matnwb/+extension/+internal/downloadExtensionRepository.m @@ -44,31 +44,3 @@ end end end - -function downloadUrl = buildRepoDownloadUrl(repositoryUrl, branchName, extensionName) -% buildRepoDownloadUrl - Build a download URL for a given repository and branch - arguments - repositoryUrl (1,1) string - branchName (1,1) string - extensionName (1,1) string - end - - if endsWith(repositoryUrl, '/') - repositoryUrl = extractBefore(repositoryUrl, strlength(repositoryUrl)); - end - if contains(repositoryUrl, 'github.com') - downloadUrl = sprintf( '%s/archive/refs/heads/%s.zip', repositoryUrl, branchName ); - - elseif contains(repositoryUrl, 'gitlab.com') - repoPathSegments = strsplit(repositoryUrl, '/'); - repoName = repoPathSegments{end}; - downloadUrl = sprintf( '%s/-/archive/%s/%s-%s.zip', ... - repositoryUrl, branchName, repoName, branchName); - - else - error('NWB:InstallExtension:UnknownRepository', ... - ['Extension "%s" is located in an unsupported repository ', ... - '/ source location. \nPlease create an issue on MatNWB''s ', ... - 'github page'], extensionName) - end -end From 67680c20561027aecd8899aa385086e070d9efeb Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 2 Jan 2025 11:53:33 +0100 Subject: [PATCH 22/47] Update docstring for nwbInstallExtension --- nwbInstallExtension.m | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index 1a9449cea..cc065788e 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -1,8 +1,8 @@ function nwbInstallExtension(extensionNames, options) -% nwbInstallExtension - Installs a specified NWB extension. +% NWBINSTALLEXTENSION - Installs a specified NWB extension. % -% Usage: -% nwbInstallExtension(extensionNames) installs Neurodata Without Borders +% Syntax: +% NWBINSTALLEXTENSION(extensionNames) installs Neurodata Without Borders % (NWB) extensions to extend the functionality of the core NWB schemas. % extensionNames is a scalar string or a string array, containing the name % of one or more extensions from the Neurodata Extensions Catalog From b2e679ac89def8a42d6a2238e411fce9c3187d8f Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 2 Jan 2025 11:57:20 +0100 Subject: [PATCH 23/47] Fix docstring indentation in nwbInstallExtension --- nwbInstallExtension.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nwbInstallExtension.m b/nwbInstallExtension.m index cc065788e..24f961f5b 100644 --- a/nwbInstallExtension.m +++ b/nwbInstallExtension.m @@ -2,10 +2,10 @@ function nwbInstallExtension(extensionNames, options) % NWBINSTALLEXTENSION - Installs a specified NWB extension. % % Syntax: -% NWBINSTALLEXTENSION(extensionNames) installs Neurodata Without Borders -% (NWB) extensions to extend the functionality of the core NWB schemas. -% extensionNames is a scalar string or a string array, containing the name -% of one or more extensions from the Neurodata Extensions Catalog +% NWBINSTALLEXTENSION(extensionNames) installs Neurodata Without Borders +% (NWB) extensions to extend the functionality of the core NWB schemas. +% extensionNames is a scalar string or a string array, containing the name +% of one or more extensions from the Neurodata Extensions Catalog % % Valid Extension Names (from https://nwb-extensions.github.io): % - "ndx-miniscope" From 69f07d92df777d988f5c56765f3aa3261e4d1852 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 2 Jan 2025 13:50:39 +0100 Subject: [PATCH 24/47] Add doc pages describing how to use (ndx) extensions --- docs/source/index.rst | 2 +- .../generating_extension_api.rst | 40 +++++++++++++++++++ .../installing_extensions.rst | 12 ++++++ .../getting_started/using_extenstions.rst | 17 ++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 docs/source/pages/getting_started/using_extensions/generating_extension_api.rst create mode 100644 docs/source/pages/getting_started/using_extensions/installing_extensions.rst create mode 100644 docs/source/pages/getting_started/using_extenstions.rst diff --git a/docs/source/index.rst b/docs/source/index.rst index 6e25d53e6..e2e952449 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -20,6 +20,7 @@ Contents pages/getting_started/installation_users pages/getting_started/important pages/getting_started/file_read + pages/getting_started/using_extenstions.rst pages/tutorials/index pages/getting_started/overview_citing @@ -38,4 +39,3 @@ Contents pages/developer/contributing pages/developer/documentation - diff --git a/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst b/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst new file mode 100644 index 000000000..8ea510ffc --- /dev/null +++ b/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst @@ -0,0 +1,40 @@ +Generating Extension API +------------------------ + +If you have created a neurodata extension or have the files for a third-party +extension locally, you can use the MatNWB function :func:`generateExtension` to +create MATLAB classes for the extension (replace the path argument with the real +path name to the namespace.yaml file): + +.. code-block:: MATLAB + + generateExtension("path/to/extension/namespace.yaml") + +The class files will be generated under the ``+types/+`` namespace in +the matnwb root directory, and can be accessed via standard MATLAB class syntax. +Assuming we have an extension called ``ndx-example`` which define a +``TetrodeSeries`` neurodata type, simply call: + +.. code-block:: MATLAB + + ts = types.ndx_example.TetrodeSeries(); + +.. important:: + Spaces are not allowed in Neurodata Extensions names, and ``-`` is used instead. + In MATLAB, any occurence of ``-`` is converted to ``_``, and in general, MatNWB + will convert namespace names if they are not valid MATLAB identifiers. See + `Variable Names `_ + for more information. In most cases, the conversion conforms with MATLAB's approach + with `matlab.lang.makeValidName() `_ + +To generate MatNWB classes in a custom location, you can use the optional ``savedir`` argument: + +.. code-block:: MATLAB + + generateExtension("path/to/ndx-example/namespace.yaml", ... + "savedir", "my/temporary/folder") + +.. note:: + Generating extensions in a custom location is generally not needed, + but is useful in advanced use cases like running tests or in other situations + where you need to better control the MATLAB search path. diff --git a/docs/source/pages/getting_started/using_extensions/installing_extensions.rst b/docs/source/pages/getting_started/using_extensions/installing_extensions.rst new file mode 100644 index 000000000..b9ab274c2 --- /dev/null +++ b/docs/source/pages/getting_started/using_extensions/installing_extensions.rst @@ -0,0 +1,12 @@ +Installing Published Extensions +------------------------------- + +In MatNWB, use the function :func:`nwbInstallExtension` to download and generate classes +for published Neurodata Extensions: + +.. code-block:: MATLAB + + nwbInstallExtension("ndx-extension") + +Replace ``ndx-extension`` with the name of an actual extension. For a complete +list of published extensions, visit the `Neurodata Extension Catalog `_. diff --git a/docs/source/pages/getting_started/using_extenstions.rst b/docs/source/pages/getting_started/using_extenstions.rst new file mode 100644 index 000000000..c15ce343d --- /dev/null +++ b/docs/source/pages/getting_started/using_extenstions.rst @@ -0,0 +1,17 @@ +Using Neurodata Extensions +========================== + +The `NWB Specification Language `_ +can be used to create Neurodata Extensions (NDX), which extend the core NWB schemas +with modified or entirely new data types. This is useful if you work with data +that has specific metadata or data requirements not covered by the core NWB schemas. +To learn more about extending NWB, see the :nwb_overview:`NWB Overview Documentation`, +and for a list of published extensions, visit the `Neurodata Extension Catalog `_. + +The following sections describe how to use extensions in MatNWB: + +.. toctree:: + :maxdepth: 2 + + using_extensions/generating_extension_api + using_extensions/installing_extensions From 07d51627c621127511566f4204ae9534e2d9784e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 2 Jan 2025 13:54:09 +0100 Subject: [PATCH 25/47] Fix typo --- .../using_extensions/generating_extension_api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst b/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst index 8ea510ffc..73f06cc12 100644 --- a/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst +++ b/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst @@ -21,7 +21,7 @@ Assuming we have an extension called ``ndx-example`` which define a .. important:: Spaces are not allowed in Neurodata Extensions names, and ``-`` is used instead. - In MATLAB, any occurence of ``-`` is converted to ``_``, and in general, MatNWB + In MATLAB, any occurrence of ``-`` is converted to ``_``, and in general, MatNWB will convert namespace names if they are not valid MATLAB identifiers. See `Variable Names `_ for more information. In most cases, the conversion conforms with MATLAB's approach From 13b0d1b3fcad98936c434a0ccf3b938cb6860e54 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 9 Jan 2025 11:02:04 +0100 Subject: [PATCH 26/47] Update +tests/+unit/InstallExtensionTest.m Co-authored-by: Ben Dichter --- +tests/+unit/InstallExtensionTest.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/+tests/+unit/InstallExtensionTest.m b/+tests/+unit/InstallExtensionTest.m index 45016b88f..db7e18260 100644 --- a/+tests/+unit/InstallExtensionTest.m +++ b/+tests/+unit/InstallExtensionTest.m @@ -43,7 +43,7 @@ function testUseInstalledExtension(testCase) 'types.ndx_miniscope.Miniscope') end - function testGetExtensionMetadata(testCase) + function testGetExtensionInfo(testCase) extensionName = "ndx-miniscope"; metadata = matnwb.extension.getExtensionInfo(extensionName); testCase.verifyClass(metadata, 'struct') From 32342ed9368c14fa967f3bc3c622b6e71742fc38 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 9 Jan 2025 11:02:32 +0100 Subject: [PATCH 27/47] Update docs/source/pages/getting_started/using_extensions/generating_extension_api.rst Co-authored-by: Ben Dichter --- .../using_extensions/generating_extension_api.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst b/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst index 73f06cc12..d2f5ca363 100644 --- a/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst +++ b/docs/source/pages/getting_started/using_extensions/generating_extension_api.rst @@ -12,8 +12,8 @@ path name to the namespace.yaml file): The class files will be generated under the ``+types/+`` namespace in the matnwb root directory, and can be accessed via standard MATLAB class syntax. -Assuming we have an extension called ``ndx-example`` which define a -``TetrodeSeries`` neurodata type, simply call: +For example, if we had an extension called ``ndx-example`` which defined a +``TetrodeSeries`` neurodata type, we would call: .. code-block:: MATLAB From b9f8f2cedd96a65c2be314a2756b22cea8bed099 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 14 Jan 2025 19:13:43 +0100 Subject: [PATCH 28/47] Add docstrings for functions to retrieve and list extension info --- +matnwb/+extension/getExtensionInfo.m | 38 ++++++++++++++++++++++++--- +matnwb/+extension/listExtensions.m | 22 ++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/+matnwb/+extension/getExtensionInfo.m b/+matnwb/+extension/getExtensionInfo.m index 411978b0c..85b4b3c86 100644 --- a/+matnwb/+extension/getExtensionInfo.m +++ b/+matnwb/+extension/getExtensionInfo.m @@ -1,5 +1,37 @@ -function metadata = getExtensionInfo(extensionName) -% getExtensionInfo - Get metadata for specified extension +function info = getExtensionInfo(extensionName) +% getExtensionInfo - Get metadata for the specified Neurodata extension +% +% Syntax: +% info = matnwb.extension.GETEXTENSIONINFO(extensionName) Returns a struct +% with metadata/information about the specified extension. The extension +% must be registered in the Neurodata Extension Catalog. +% +% Input Arguments: +% - extensionName (string) - +% Name of a Neurodata Extension, e.g "ndx-miniscope". +% +% Output Arguments: +% - info (struct) - +% Struct with metadata / information for the specified extension. The struct +% has the following fields +% - name : The name of the extension. +% - version : The current version of the extension. +% - last_updated : A timestamp indicating when the extension was last updated. +% - src : The URL to the source repository or homepage of the extension. +% - license : The license type under which the extension is distributed. +% - maintainers : A cell array or array of strings listing the maintainers. +% - readme : A string containing the README documentation or description. +% +% Usage: +% Example 1 - Retrieve and display information for the 'ndx-miniscope' extension:: +% +% info = matnwb.extension.getExtensionInfo('ndx-miniscope'); +% +% % Display the version of the extension. +% fprintf('Extension version: %s\n', info.version); +% +% See also: +% matnwb.extension.listExtensions arguments extensionName (1,1) string @@ -12,5 +44,5 @@ any(isMatch), ... 'NWB:DisplayExtensionMetadata:ExtensionNotFound', ... 'Extension "%s" was not found in the extension catalog:\n%s', extensionName, extensionList) - metadata = table2struct(T(isMatch, :)); + info = table2struct(T(isMatch, :)); end diff --git a/+matnwb/+extension/listExtensions.m b/+matnwb/+extension/listExtensions.m index e8ae59c1b..56821dcb7 100644 --- a/+matnwb/+extension/listExtensions.m +++ b/+matnwb/+extension/listExtensions.m @@ -1,5 +1,27 @@ function extensionTable = listExtensions(options) +% listExtensions - List available extensions in the Neurodata Extension Catalog +% +% Syntax: +% extensionTable = matnwb.extension.listExtensions() returns a table where +% each row holds information about a registered extension. +% +% Output Arguments: +% - extensionTable (table) - +% Table of metadata / information for each registered extension. The table +% has the following columns +% - name : The name of the extension. +% - version : The current version of the extension. +% - last_updated : A timestamp indicating when the extension was last updated. +% - src : The URL to the source repository or homepage of the extension. +% - license : The license type under which the extension is distributed. +% - maintainers : A cell array or array of strings listing the maintainers. +% - readme : A string containing the README documentation or description. +% +% See also: matnwb.extension.getExtensionInfo + arguments + % Refresh - Flag to refresh the catalog (Only relevant if the + % remote catalog has been updated). options.Refresh (1,1) logical = false end From 2b0e820edc59bd48b2fd5cab8e8b891e20fafa02 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 14 Jan 2025 22:00:43 +0100 Subject: [PATCH 29/47] Fix docstring formatting/whitespace --- +matnwb/+extension/getExtensionInfo.m | 31 ++++++++++++++------------- +matnwb/+extension/listExtensions.m | 23 ++++++++++---------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/+matnwb/+extension/getExtensionInfo.m b/+matnwb/+extension/getExtensionInfo.m index 85b4b3c86..830077900 100644 --- a/+matnwb/+extension/getExtensionInfo.m +++ b/+matnwb/+extension/getExtensionInfo.m @@ -3,8 +3,8 @@ % % Syntax: % info = matnwb.extension.GETEXTENSIONINFO(extensionName) Returns a struct -% with metadata/information about the specified extension. The extension -% must be registered in the Neurodata Extension Catalog. +% with metadata/information about the specified extension. The extension +% must be registered in the Neurodata Extension Catalog. % % Input Arguments: % - extensionName (string) - @@ -13,22 +13,23 @@ % Output Arguments: % - info (struct) - % Struct with metadata / information for the specified extension. The struct -% has the following fields -% - name : The name of the extension. -% - version : The current version of the extension. -% - last_updated : A timestamp indicating when the extension was last updated. -% - src : The URL to the source repository or homepage of the extension. -% - license : The license type under which the extension is distributed. -% - maintainers : A cell array or array of strings listing the maintainers. -% - readme : A string containing the README documentation or description. +% has the following fields: +% +% - name - The name of the extension. +% - version - The current version of the extension. +% - last_updated - A timestamp indicating when the extension was last updated. +% - src - The URL to the source repository or homepage of the extension. +% - license - The license type under which the extension is distributed. +% - maintainers - A cell array or array of strings listing the maintainers. +% - readme - A string containing the README documentation or description. % % Usage: % Example 1 - Retrieve and display information for the 'ndx-miniscope' extension:: -% -% info = matnwb.extension.getExtensionInfo('ndx-miniscope'); -% -% % Display the version of the extension. -% fprintf('Extension version: %s\n', info.version); +% +% info = matnwb.extension.getExtensionInfo('ndx-miniscope'); +% +% % Display the version of the extension. +% fprintf('Extension version: %s\n', info.version); % % See also: % matnwb.extension.listExtensions diff --git a/+matnwb/+extension/listExtensions.m b/+matnwb/+extension/listExtensions.m index 56821dcb7..c2d68e607 100644 --- a/+matnwb/+extension/listExtensions.m +++ b/+matnwb/+extension/listExtensions.m @@ -2,20 +2,21 @@ % listExtensions - List available extensions in the Neurodata Extension Catalog % % Syntax: -% extensionTable = matnwb.extension.listExtensions() returns a table where -% each row holds information about a registered extension. +% extensionTable = matnwb.extension.LISTEXTENSIONS() returns a table where +% each row holds information about a registered extension. % -% Output Arguments: +% Output Arguments: % - extensionTable (table) - % Table of metadata / information for each registered extension. The table -% has the following columns -% - name : The name of the extension. -% - version : The current version of the extension. -% - last_updated : A timestamp indicating when the extension was last updated. -% - src : The URL to the source repository or homepage of the extension. -% - license : The license type under which the extension is distributed. -% - maintainers : A cell array or array of strings listing the maintainers. -% - readme : A string containing the README documentation or description. +% has the following columns: +% +% - name - The name of the extension. +% - version - The current version of the extension. +% - last_updated - A timestamp indicating when the extension was last updated. +% - src - The URL to the source repository or homepage of the extension. +% - license - The license type under which the extension is distributed. +% - maintainers - A cell array or array of strings listing the maintainers. +% - readme - A string containing the README documentation or description. % % See also: matnwb.extension.getExtensionInfo From bcb25847bc416b356adb4e7a9a8cd83d602d1035 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 14 Jan 2025 22:43:54 +0100 Subject: [PATCH 30/47] Update listExtensions.m Add example to docstring --- +matnwb/+extension/listExtensions.m | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/+matnwb/+extension/listExtensions.m b/+matnwb/+extension/listExtensions.m index c2d68e607..b93c2f718 100644 --- a/+matnwb/+extension/listExtensions.m +++ b/+matnwb/+extension/listExtensions.m @@ -18,7 +18,14 @@ % - maintainers - A cell array or array of strings listing the maintainers. % - readme - A string containing the README documentation or description. % -% See also: matnwb.extension.getExtensionInfo +% Usage: +% Example 1 - List and display extensions:: +% +% T = matnwb.extension.listExtensions(); +% disp(T) +% +% See also: +% matnwb.extension.getExtensionInfo arguments % Refresh - Flag to refresh the catalog (Only relevant if the From 691ef81591f36920b5a63cea3b4c34c5526d9734 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 15:41:58 +0100 Subject: [PATCH 31/47] Move static test methods into io.internal.h5 namespace Introduce some functions that will be useful later --- +io/+internal/+h5/deleteAttribute.m | 20 ++++++++++ +io/+internal/+h5/deleteGroup.m | 16 ++++++++ +io/+internal/+h5/mustBeH5File.m | 17 ++++++++ +io/+internal/+h5/mustBeH5FileReference.m | 15 ++++++++ +io/+internal/+h5/openFile.m | 46 ++++++++++++++++++++++ +io/+internal/+h5/openGroup.m | 13 +++++++ +io/+internal/+h5/openObject.m | 13 +++++++ +io/+internal/+h5/resolveFileReference.m | 30 +++++++++++++++ +io/+internal/+h5/validateLocation.m | 9 +++++ +tests/+system/NWBFileIOTest.m | 47 ++++------------------- 10 files changed, 186 insertions(+), 40 deletions(-) create mode 100644 +io/+internal/+h5/deleteAttribute.m create mode 100644 +io/+internal/+h5/deleteGroup.m create mode 100644 +io/+internal/+h5/mustBeH5File.m create mode 100644 +io/+internal/+h5/mustBeH5FileReference.m create mode 100644 +io/+internal/+h5/openFile.m create mode 100644 +io/+internal/+h5/openGroup.m create mode 100644 +io/+internal/+h5/openObject.m create mode 100644 +io/+internal/+h5/resolveFileReference.m create mode 100644 +io/+internal/+h5/validateLocation.m diff --git a/+io/+internal/+h5/deleteAttribute.m b/+io/+internal/+h5/deleteAttribute.m new file mode 100644 index 000000000..e973929ed --- /dev/null +++ b/+io/+internal/+h5/deleteAttribute.m @@ -0,0 +1,20 @@ +function deleteAttribute(fileReference, objectLocation, attributeName) +% deleteAttribute - Delete the specified attribute from an NWB file + + arguments + fileReference {io.internal.h5.mustBeH5FileReference} + objectLocation (1,1) string + attributeName (1,1) string + end + + objectLocation = io.internal.h5.validateLocation(objectLocation); + + % Open the HDF5 file in read-write mode + [fileId, fileCleanupObj] = io.internal.h5.resolveFileReference(fileReference, "w"); %#ok + + % Open the object (dataset or group) + [objectId, objectCleanupObj] = io.internal.h5.openObject(fileId, objectLocation); %#ok + + % Delete the attribute + H5A.delete(objectId, attributeName); +end diff --git a/+io/+internal/+h5/deleteGroup.m b/+io/+internal/+h5/deleteGroup.m new file mode 100644 index 000000000..c6eed4262 --- /dev/null +++ b/+io/+internal/+h5/deleteGroup.m @@ -0,0 +1,16 @@ +function deleteGroup(fileReference, groupLocation) +% deleteGroup - Delete the specified group from an NWB file + + arguments + fileReference {io.internal.h5.mustBeH5FileReference} + groupLocation (1,1) string + end + + groupLocation = io.internal.h5.validateLocation(groupLocation); + + % Open the HDF5 file in read-write mode + [fileId, fileCleanupObj] = io.internal.h5.resolveFileReference(fileReference, "w"); %#ok + + % Delete the group + H5L.delete(fileId, groupLocation, 'H5P_DEFAULT'); +end diff --git a/+io/+internal/+h5/mustBeH5File.m b/+io/+internal/+h5/mustBeH5File.m new file mode 100644 index 000000000..25794de97 --- /dev/null +++ b/+io/+internal/+h5/mustBeH5File.m @@ -0,0 +1,17 @@ +function mustBeH5File(value) + arguments + value {mustBeFile} + end + + VALID_FILE_ENDING = ["h5", "nwb"]; + validExtensions = "." + VALID_FILE_ENDING; + + hasH5Extension = endsWith(value, validExtensions, 'IgnoreCase', true); + + if ~hasH5Extension + exception = MException(... + 'MatNWB:validators:mustBeH5File', ... + 'Expected file "%s" to have .h5 or .nwb file extension', value); + throwAsCaller(exception) + end +end diff --git a/+io/+internal/+h5/mustBeH5FileReference.m b/+io/+internal/+h5/mustBeH5FileReference.m new file mode 100644 index 000000000..ec3f5dd3a --- /dev/null +++ b/+io/+internal/+h5/mustBeH5FileReference.m @@ -0,0 +1,15 @@ +function mustBeH5FileReference(value) + arguments + value {mustBeA(value, ["char", "string", "H5ML.id"])} + end + + if isa(value, "char") || isa(value, "string") + try + io.internal.h5.mustBeH5File(value) + catch ME + throwAsCaller(ME) + end + else + % value is a H5ML.id, ok! + end +end diff --git a/+io/+internal/+h5/openFile.m b/+io/+internal/+h5/openFile.m new file mode 100644 index 000000000..04f212b93 --- /dev/null +++ b/+io/+internal/+h5/openFile.m @@ -0,0 +1,46 @@ +function [fileId, fileCleanupObj] = openFile(fileName, permission) +% openFile Opens an HDF5 file with the specified permissions and ensures cleanup. +% +% [fileId, fileCleanupObj] = io.internal.h5.openFile(fileName) opens the HDF5 +% file specified by fileName in read-only mode ('r') by default. +% +% [fileId, fileCleanupObj] = io.internal.h5.openFile(fileName, permission) +% opens the HDF5 file specified by fileName with the access mode defined by +% permission. +% +% Input Arguments: +% fileName - A string or character vector specifying the path to the +% HDF5 file. This must be a .h5 or .nwb file. +% +% permission - (Optional) A scalar string specifying the file access mode. +% Valid values are "r" for read-only (default) and "w" for +% read-write. +% +% Output Arguments: +% fileId - The file identifier returned by H5F.open, used to +% reference the open file. +% +% fileCleanupObj - A cleanup object (onCleanup) that ensures the file is +% closed automatically when fileCleanupObj goes out of +% scope. +% +% Example: +% [fid, cleanupObj] = io.internal.h5.openFile("data.h5", "w"); +% % Use fid for file operations. +% % When cleanupObj is cleared or goes out of scope, the file is +% % automatically closed. + + arguments + fileName {io.internal.h5.mustBeH5File} + permission (1,1) string {mustBeMember(permission, ["r", "w"])} = "r" + end + + switch permission + case "r" + accessFlag = 'H5F_ACC_RDONLY'; + case "w" + accessFlag = 'H5F_ACC_RDWR'; + end + fileId = H5F.open(fileName, accessFlag, 'H5P_DEFAULT'); + fileCleanupObj = onCleanup(@(fid) H5F.close(fileId)); +end diff --git a/+io/+internal/+h5/openGroup.m b/+io/+internal/+h5/openGroup.m new file mode 100644 index 000000000..8edc6ad67 --- /dev/null +++ b/+io/+internal/+h5/openGroup.m @@ -0,0 +1,13 @@ +function [groupId, groupCleanupObj] = openGroup(fileId, h5Location) +% openGroup Opens an HDF5 group at given location and ensures cleanup. + + arguments + fileId {mustBeA(fileId, "H5ML.id")} + h5Location (1,1) string + end + + % Open the specified location (group) + groupLocation = io.internal.h5.validateLocation(h5Location); + groupId = H5G.open(fileId, groupLocation); + groupCleanupObj = onCleanup(@(gid) H5G.close(groupId)); +end diff --git a/+io/+internal/+h5/openObject.m b/+io/+internal/+h5/openObject.m new file mode 100644 index 000000000..8f15220ba --- /dev/null +++ b/+io/+internal/+h5/openObject.m @@ -0,0 +1,13 @@ +function [objectId, objectCleanupObj] = openObject(fileId, objectLocation) +% openObject Opens an HDF5 object at given location and ensures cleanup. + + arguments + fileId {mustBeA(fileId, "H5ML.id")} + objectLocation (1,1) string + end + + % Open the object (dataset or group) + objectLocation = io.internal.h5.validateLocation(objectLocation); + objectId = H5O.open(fileId, objectLocation, 'H5P_DEFAULT'); + objectCleanupObj = onCleanup(@(oid) H5O.close(objectId)); +end diff --git a/+io/+internal/+h5/resolveFileReference.m b/+io/+internal/+h5/resolveFileReference.m new file mode 100644 index 000000000..58e225b06 --- /dev/null +++ b/+io/+internal/+h5/resolveFileReference.m @@ -0,0 +1,30 @@ +function [h5FileId, fileCleanupObj] = resolveFileReference(fileReference, permission) +% resolveFileReference - Resolve a file reference to a H5 File ID. +% +% Utility method to resolve a file reference, which can be either a +% filepath or a file id for a h5 file. +% +% The returned value will always be a file ID. This allows functions that +% does operations on h5 files to receive either a file path or a file id +% +% Note: If the file reference is a file ID for an open file, the permission +% might be different than the provided/requested permission. + + arguments + fileReference {io.internal.h5.mustBeH5FileReference} + permission (1,1) string {mustBeMember(permission, ["r", "w"])} = "r" + end + + if isa(fileReference, "char") || isa(fileReference, "string") + % Need to open the file + if isfile(fileReference) + [h5FileId, fileCleanupObj] = io.internal.h5.openFile(fileReference, permission); + else + error('File "%s" does not exist', fileReference) + end + else + h5FileId = fileReference; + % If the file is already open, we are not responsible for closing it + fileCleanupObj = []; + end +end diff --git a/+io/+internal/+h5/validateLocation.m b/+io/+internal/+h5/validateLocation.m new file mode 100644 index 000000000..754f1fcb1 --- /dev/null +++ b/+io/+internal/+h5/validateLocation.m @@ -0,0 +1,9 @@ +function locationName = validateLocation(locationName) + arguments + locationName (1,1) string + end + + if ~startsWith(locationName, "/") + locationName = "/" + locationName; + end +end diff --git a/+tests/+system/NWBFileIOTest.m b/+tests/+system/NWBFileIOTest.m index 03a382559..2772a4857 100644 --- a/+tests/+system/NWBFileIOTest.m +++ b/+tests/+system/NWBFileIOTest.m @@ -58,15 +58,17 @@ function readFileWithoutSpec(testCase) fileName = ['MatNWB.' testCase.className() '.testReadFileWithoutSpec.nwb']; nwbExport(testCase.file, fileName) - testCase.deleteGroupFromFile(fileName, 'specifications') + io.internal.h5.deleteGroup(fileName, 'specifications') + nwbRead(fileName); end function readFileWithoutSpecLoc(testCase) + fileName = ['MatNWB.' testCase.className() '.testReadFileWithoutSpecLoc.nwb']; nwbExport(testCase.file, fileName) - testCase.deleteAttributeFromFile(fileName, '/', '.specloc') + io.internal.h5.deleteAttribute(fileName, '/', '.specloc') % When specloc is missing, the specifications are not added to % the blacklist, so it will get passed as an input to NwbFile. @@ -77,7 +79,7 @@ function readFileWithUnsupportedVersion(testCase) fileName = ['MatNWB.' testCase.className() '.testReadFileWithUnsupportedVersion.nwb']; nwbExport(testCase.file, fileName) - testCase.deleteAttributeFromFile(fileName, '/', 'nwb_version') + io.internal.h5.deleteAttribute(fileName, '/', 'nwb_version') file_id = H5F.open(fileName, 'H5F_ACC_RDWR', 'H5P_DEFAULT'); io.writeAttribute(file_id, '/nwb_version', '1.0.0') @@ -93,8 +95,8 @@ function readFileWithUnsupportedVersionAndNoSpecloc(testCase) fileName = ['MatNWB.' testCase.className() '.testReadFileWithUnsupportedVersionAndNoSpecloc.nwb']; nwbExport(testCase.file, fileName) - testCase.deleteAttributeFromFile(fileName, '/', '.specloc') - testCase.deleteAttributeFromFile(fileName, '/', 'nwb_version') + io.internal.h5.deleteAttribute(fileName, '/', '.specloc') + io.internal.h5.deleteAttribute(fileName, '/', 'nwb_version') file_id = H5F.open(fileName, 'H5F_ACC_RDWR', 'H5P_DEFAULT'); io.writeAttribute(file_id, '/nwb_version', '1.0.0') @@ -105,39 +107,4 @@ function readFileWithUnsupportedVersionAndNoSpecloc(testCase) testCase.verifyError(@(fn) nwbRead(fileName), 'MATLAB:TooManyInputs'); end end - - methods (Static, Access = private) - function deleteGroupFromFile(fileName, groupName) - if ~startsWith(groupName, '/') - groupName = ['/', groupName]; - end - - % Open the HDF5 file in read-write mode - file_id = H5F.open(fileName, 'H5F_ACC_RDWR', 'H5P_DEFAULT'); - - % Delete the group - H5L.delete(file_id, groupName, 'H5P_DEFAULT'); - - % Close the HDF5 file - H5F.close(file_id); - end - - function deleteAttributeFromFile(fileName, objectName, attributeName) - % Open the HDF5 file in read-write mode - file_id = H5F.open(fileName, 'H5F_ACC_RDWR', 'H5P_DEFAULT'); - - % Open the object (dataset or group) - object_id = H5O.open(file_id, objectName, 'H5P_DEFAULT'); - - % Delete the attribute - H5A.delete(object_id, attributeName); - - % Close the object - H5O.close(object_id); - - % Close the HDF5 file - H5F.close(file_id); - end - end end - From 5958136fb9f377e9ab724453ed2f86b7d2ecdc38 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 15:42:30 +0100 Subject: [PATCH 32/47] Update writeEmbeddedSpecifications.m Add arguments block, fix function name --- +io/+spec/writeEmbeddedSpecifications.m | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/+io/+spec/writeEmbeddedSpecifications.m b/+io/+spec/writeEmbeddedSpecifications.m index 23c2e269f..2c5396f3a 100644 --- a/+io/+spec/writeEmbeddedSpecifications.m +++ b/+io/+spec/writeEmbeddedSpecifications.m @@ -1,4 +1,11 @@ function writeEmbeddedSpecifications(fid, jsonSpecs) +% writeEmbeddedSpecifications - Write schema specifications to an NWB file + + arguments + fid % File id for a h5 file + jsonSpecs % String representation of schema specifications in json format + end + specLocation = io.spec.internal.readEmbeddedSpecLocation(fid); if isempty(specLocation) @@ -37,8 +44,8 @@ function writeEmbeddedSpecifications(fid, jsonSpecs) function versionNames = getVersionNames(namespaceGroupId) [~, ~, versionNames] = H5L.iterate(namespaceGroupId,... 'H5_INDEX_NAME', 'H5_ITER_NATIVE',... - 0, @removeGroups, {}); - function [status, versionNames] = removeGroups(~, name, versionNames) + 0, @appendName, {}); + function [status, versionNames] = appendName(~, name, versionNames) versionNames{end+1} = name; status = 0; end From f5af434c4b3edd6dad8755560c05d79176dd1cd9 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 16:13:33 +0100 Subject: [PATCH 33/47] Add validateEmbeddedSpecifications --- +io/+internal/+h5/listGroupNames.m | 28 +++++++++++++ +io/+spec/validateEmbeddedSpecifications.m | 48 ++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 +io/+internal/+h5/listGroupNames.m create mode 100644 +io/+spec/validateEmbeddedSpecifications.m diff --git a/+io/+internal/+h5/listGroupNames.m b/+io/+internal/+h5/listGroupNames.m new file mode 100644 index 000000000..12b6048a6 --- /dev/null +++ b/+io/+internal/+h5/listGroupNames.m @@ -0,0 +1,28 @@ +function groupNames = listGroupNames(fileReference, h5Location) + + arguments + fileReference {io.internal.h5.mustBeH5FileReference} + h5Location (1,1) string + end + + [fileId, fileCleanupObj] = io.internal.h5.resolveFileReference(fileReference); %#ok + + % Open the specified location (group) + [groupId, groupCleanupObj] = io.internal.h5.openGroup(fileId, h5Location); %#ok + + % Use H5L.iterate to iterate over the links + [~, ~, groupNames] = H5L.iterate(... + groupId, "H5_INDEX_NAME", "H5_ITER_INC", 0, @collectGroupNames, {}); + + % Define iteration function + function [status, groupNames] = collectGroupNames(groupId, name, groupNames) + % Only retrieve name of groups + objId = H5O.open(groupId, name, 'H5P_DEFAULT'); + objInfo = H5O.get_info(objId); + if objInfo.type == H5ML.get_constant_value('H5O_TYPE_GROUP') + groupNames{end+1} = name; + end + H5O.close(objId); + status = 0; % Continue iteration + end +end diff --git a/+io/+spec/validateEmbeddedSpecifications.m b/+io/+spec/validateEmbeddedSpecifications.m new file mode 100644 index 000000000..56a8f83e0 --- /dev/null +++ b/+io/+spec/validateEmbeddedSpecifications.m @@ -0,0 +1,48 @@ +function validateEmbeddedSpecifications(h5_file_id, expectedNamespaceNames) +% validateEmbeddedSpecifications - Validate the embedded specifications +% +% This function does two things: +% 1) Displays a warning if specifications of expected namespaces +% are not embedded in the file. +% E.g if cached namespaces were cleared prior to export. +% +% 2) Deletes specifications for unused namespaces that are embedded. +% - E.g. If neurodata type from an embedded namespace was removed and the +% file was re-exported + +% NB: Input h5_file_id must point to a file opened with write access + + specLocation = io.spec.internal.readEmbeddedSpecLocation(h5_file_id); + embeddedNamespaceNames = io.internal.h5.listGroupNames(h5_file_id, specLocation); + + checkMissingNamespaces(expectedNamespaceNames, embeddedNamespaceNames) + + unusedNamespaces = checkUnusedNamespaces(... + expectedNamespaceNames, embeddedNamespaceNames); + + if ~isempty(unusedNamespaces) + deleteUnusedNamespaces(h5_file_id, unusedNamespaces, specLocation) + end +end + +function checkMissingNamespaces(expectedNamespaceNames, embeddedNamespaceNames) +% checkMissingNamespaces - Check if any namespace specs are missing from the file + missingNamespaces = setdiff(expectedNamespaceNames, embeddedNamespaceNames); + if ~isempty(missingNamespaces) + missingNamespacesStr = strjoin(" " + string(missingNamespaces), newline); + warning('Namespace is missing:\n%s', missingNamespacesStr) + end +end + +function unusedNamespaces = checkUnusedNamespaces(expectedNamespaceNames, embeddedNamespaceNames) +% checkUnusedNamespaces - Check if any namespace specs in the file are unused + unusedNamespaces = setdiff(embeddedNamespaceNames, expectedNamespaceNames); +end + +function deleteUnusedNamespaces(fileId, unusedNamespaces, specRootLocation) + for i = 1:numel(unusedNamespaces) + thisName = unusedNamespaces{i}; + namespaceSpecLocation = strjoin( {specRootLocation, thisName}, '/'); + io.internal.h5.deleteGroup(fileId, namespaceSpecLocation) + end +end From 9de778bb40defe8f71431244f46c80d3495ef89a Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 16:15:55 +0100 Subject: [PATCH 34/47] Update NwbFile.m Redefine listNwbTypes method, add validation of embedded namespaces --- NwbFile.m | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/NwbFile.m b/NwbFile.m index 189f61a37..c0a3839bf 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -118,8 +118,13 @@ function export(obj, filename, mode) varargin{:}); end - function nwbTypeNames = listNwbTypes(obj) - % listNwbTypes - List all unique NWB types in file + function nwbTypeNames = listNwbTypes(obj, options) + % listNwbTypes - List all unique NWB (neurodata) types in file + arguments + obj (1,1) NwbFile + options.IncludeParentTypes (1,1) logical = false + end + objectMap = searchProperties(containers.Map, obj, '', ''); objects = objectMap.values(); @@ -130,6 +135,15 @@ function export(obj, filename, mode) ignore = startsWith(objectClassNames, "types.untyped"); nwbTypeNames = objectClassNames(keep & ~ignore); + + if options.IncludeParentTypes + includedNwbTypesWithParents = string.empty; + for i = 1:numel(nwbTypeNames) + typeHierarchy = schemes.utility.listNwbTypeHierarchy(nwbTypeNames{i}); + includedNwbTypesWithParents = [includedNwbTypesWithParents, typeHierarchy]; %#ok + end + nwbTypeNames = includedNwbTypesWithParents; + end end end @@ -139,17 +153,13 @@ function embedSpecifications(obj, output_file_id) jsonSpecs = schemes.exportJson(); % Resolve the name of all types and parent types that are - % included in this file, in order to only include the specs for - % namespaces of types that are included in the file. - includedNwbTypes = obj.listNwbTypes(); - includedNwbTypesWithParents = string.empty; - for i = 1:numel(includedNwbTypes) - typeHierarchy = schemes.utility.listNwbTypeHierarchy(includedNwbTypes{i}); - includedNwbTypesWithParents = [includedNwbTypesWithParents, typeHierarchy]; %#ok - end + % included in this file. This will be used to filter the specs + % to embed, so that only specs with used neurodata types are + % embedded. + includedNeurodataTypes = obj.listNwbTypes("IncludeParentTypes", true); % Get the namespace names - namespaceNames = getNamespacesOfTypes(includedNwbTypesWithParents); + namespaceNames = getNamespacesForDataTypes(includedNeurodataTypes); % In the specs, the hyphen (-) is used as a word separator, while in % matnwb the underscore (_) is used. Translate names here: @@ -157,7 +167,13 @@ function embedSpecifications(obj, output_file_id) [~, keepIdx] = intersect(allMatlabNamespaceNames, namespaceNames, 'stable'); jsonSpecs = jsonSpecs(keepIdx); - io.spec.writeEmbeddedSpecifications(output_file_id, jsonSpecs); + io.spec.writeEmbeddedSpecifications(... + output_file_id, ... + jsonSpecs); + + io.spec.validateEmbeddedSpecifications(... + output_file_id, ... + strrep(namespaceNames, '_', '-')) end function resolveReferences(obj, fid, references) @@ -255,7 +271,7 @@ function resolveReferences(obj, fid, references) end end -function namespaceNames = getNamespacesOfTypes(nwbTypeNames) +function namespaceNames = getNamespacesForDataTypes(nwbTypeNames) % getNamespacesOfTypes - Get namespace names for a list of nwb types arguments nwbTypeNames (1,:) string From 258b8dcaa92bb43e68fb81bf07b4444823b9391b Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 16:45:00 +0100 Subject: [PATCH 35/47] Create listEmbeddedSpecNamespaces.m --- +io/+spec/listEmbeddedSpecNamespaces.m | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 +io/+spec/listEmbeddedSpecNamespaces.m diff --git a/+io/+spec/listEmbeddedSpecNamespaces.m b/+io/+spec/listEmbeddedSpecNamespaces.m new file mode 100644 index 000000000..9310bc577 --- /dev/null +++ b/+io/+spec/listEmbeddedSpecNamespaces.m @@ -0,0 +1,11 @@ +function namespaceNames = listEmbeddedSpecNamespaces(fileReference) + + arguments + fileReference {io.internal.h5.mustBeH5FileReference} + end + + [fileId, fileCleanupObj] = io.internal.h5.resolveFileReference(fileReference); %#ok + + specLocation = io.spec.internal.readEmbeddedSpecLocation(fileId); + namespaceNames = io.internal.h5.listGroupNames(fileId, specLocation); +end From c694d10201730f5fc1e61335f5e13b0d0a197c47 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 16:48:08 +0100 Subject: [PATCH 36/47] Update nwbExportTest.m --- +tests/+unit/nwbExportTest.m | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/+tests/+unit/nwbExportTest.m b/+tests/+unit/nwbExportTest.m index 4f25e78e3..7276cd09d 100644 --- a/+tests/+unit/nwbExportTest.m +++ b/+tests/+unit/nwbExportTest.m @@ -73,6 +73,40 @@ function testExportTimeseriesWithoutStartingTimeRate(testCase) nwbFilePath = fullfile(testCase.OutputFolder, 'test_part1.nwb'); testCase.verifyError(@(f, fn) nwbExport(testCase.NwbObject, nwbFilePath), 'NWB:CustomConstraintUnfulfilled') end + + function testEmbeddedSpecs(testCase) + + nwbFileName = 'testEmbeddedSpecs.nwb'; + + % Install extension. + %nwbInstallExtension(["ndx-miniscope", "ndx-photostim"]) + + % Export a file not using a type from an extension + nwb = testCase.initNwbFile(); + + nwbExport(nwb, nwbFileName); + embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); + testCase.verifyEmpty(embeddedNamespaces) + + ts = types.core.TimeSeries('data', rand(1,10), 'timestamps', 1:10); + nwb.acquisition.set('test', ts) + + nwbExport(nwb, nwbFileName); + embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); + + % Verify that extension namespace is not part of embedded specs + testCase.verifyEqual(sort(embeddedNamespaces), {'core', 'hdmf-common'}) + + % Add type for extension. + testDevice = types.ndx_photostim.Laser('model', 'Spectra-Physics'); + nwb.general_devices.set('TestDevice', testDevice) + + nwbExport(nwb, nwbFileName); + embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); + + % Verify that extension namespace is part of embedded specs. + testCase.verifyEqual(sort(embeddedNamespaces), {'core', 'hdmf-common', 'ndx-photostim'}) + end end methods (Static) From 8f0ec2898598f7cedaf929ab277f46f0aff5ec24 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 16 Jan 2025 17:00:17 +0100 Subject: [PATCH 37/47] Update test for spec/namespace embedding --- +io/+spec/validateEmbeddedSpecifications.m | 2 +- +tests/+unit/nwbExportTest.m | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/+io/+spec/validateEmbeddedSpecifications.m b/+io/+spec/validateEmbeddedSpecifications.m index 56a8f83e0..eadd9e90c 100644 --- a/+io/+spec/validateEmbeddedSpecifications.m +++ b/+io/+spec/validateEmbeddedSpecifications.m @@ -30,7 +30,7 @@ function checkMissingNamespaces(expectedNamespaceNames, embeddedNamespaceNames) missingNamespaces = setdiff(expectedNamespaceNames, embeddedNamespaceNames); if ~isempty(missingNamespaces) missingNamespacesStr = strjoin(" " + string(missingNamespaces), newline); - warning('Namespace is missing:\n%s', missingNamespacesStr) + warning('NWB:validators:MissingEmbeddedNamespace', 'Namespace is missing:\n%s', missingNamespacesStr) end end diff --git a/+tests/+unit/nwbExportTest.m b/+tests/+unit/nwbExportTest.m index 7276cd09d..0590d17e3 100644 --- a/+tests/+unit/nwbExportTest.m +++ b/+tests/+unit/nwbExportTest.m @@ -79,7 +79,7 @@ function testEmbeddedSpecs(testCase) nwbFileName = 'testEmbeddedSpecs.nwb'; % Install extension. - %nwbInstallExtension(["ndx-miniscope", "ndx-photostim"]) + nwbInstallExtension(["ndx-miniscope", "ndx-photostim"], 'savedir', '.') % Export a file not using a type from an extension nwb = testCase.initNwbFile(); @@ -99,13 +99,26 @@ function testEmbeddedSpecs(testCase) % Add type for extension. testDevice = types.ndx_photostim.Laser('model', 'Spectra-Physics'); - nwb.general_devices.set('TestDevice', testDevice) + nwb.general_devices.set('TestDevice', testDevice); nwbExport(nwb, nwbFileName); embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); % Verify that extension namespace is part of embedded specs. testCase.verifyEqual(sort(embeddedNamespaces), {'core', 'hdmf-common', 'ndx-photostim'}) + + nwb.general_devices.remove('TestDevice'); + nwbExport(nwb, nwbFileName); + + embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); + testCase.verifyEqual(sort(embeddedNamespaces), {'core', 'hdmf-common'}) + + % Test that warning for missing namespace works + [fileId, fileCleanupObj] = io.internal.h5.openFile(nwbFileName); %#ok + expectedNamespaces = {'core', 'hdmf-common', 'ndx-photostim'}; + testCase.verifyWarning( ... + @(fid,names) io.spec.validateEmbeddedSpecifications(fileId, expectedNamespaces), ... + 'NWB:validators:MissingEmbeddedNamespace') end end From ab79e41f745048c1a46d7a6c20a1ecd32c824f1a Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 22 Jan 2025 11:16:49 +0100 Subject: [PATCH 38/47] Update read_indexed_column.m --- +util/read_indexed_column.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/+util/read_indexed_column.m b/+util/read_indexed_column.m index 34e0c9c6d..e70d6e4fb 100644 --- a/+util/read_indexed_column.m +++ b/+util/read_indexed_column.m @@ -1,5 +1,5 @@ function data = read_indexed_column(vector_index, vector_data, row) error('NWB:read_indexed_column', ... - ['The utility function read_indexed_column has been reprecated. Please,' ... + ['The utility function read_indexed_column has been deprecated. Please,' ... ' use the getRow method of DynamicTable objects instead'] ... -) \ No newline at end of file +) From 7a32b7cc0277e526ee7d45845f778495786d5650 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 22 Jan 2025 23:26:39 +0100 Subject: [PATCH 39/47] Add disclaimer in deleteGroup function --- +io/+internal/+h5/deleteGroup.m | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/+io/+internal/+h5/deleteGroup.m b/+io/+internal/+h5/deleteGroup.m index c6eed4262..6755b3116 100644 --- a/+io/+internal/+h5/deleteGroup.m +++ b/+io/+internal/+h5/deleteGroup.m @@ -1,5 +1,17 @@ function deleteGroup(fileReference, groupLocation) % deleteGroup - Delete the specified group from an NWB file +% +% NB NB NB: Deleting groups & datasets from an HDF5 file does not free up space +% +% HDF5 files use a structured format to store data in hierarchical groups and +% datasets. Internally, the file maintains a structure similar to a filesystem, +% with metadata pointing to the actual data blocks. +% +% Implication: When you delete a group or dataset in an HDF5 file, the metadata +% entries for that group or dataset are removed, so they are no longer accessible. +% However, the space previously occupied by the actual data is not reclaimed or +% reused by default. This is because HDF5 does not automatically reorganize or +% compress the file when items are deleted. arguments fileReference {io.internal.h5.mustBeH5FileReference} From d97c83e7ba85d9e436476314291a33d6ac81774d Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 30 Jan 2025 18:10:46 +0100 Subject: [PATCH 40/47] Update read_indexed_column.m --- +util/read_indexed_column.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/+util/read_indexed_column.m b/+util/read_indexed_column.m index 34e0c9c6d..e70d6e4fb 100644 --- a/+util/read_indexed_column.m +++ b/+util/read_indexed_column.m @@ -1,5 +1,5 @@ function data = read_indexed_column(vector_index, vector_data, row) error('NWB:read_indexed_column', ... - ['The utility function read_indexed_column has been reprecated. Please,' ... + ['The utility function read_indexed_column has been deprecated. Please,' ... ' use the getRow method of DynamicTable objects instead'] ... -) \ No newline at end of file +) From 863d319af072e2cc3f0702efc862ef2b360f1ef8 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 30 Jan 2025 19:25:30 +0100 Subject: [PATCH 41/47] Fix broken test --- +tests/+unit/FunctionTests.m | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/+tests/+unit/FunctionTests.m b/+tests/+unit/FunctionTests.m index 2e739c5c7..3a77bc278 100644 --- a/+tests/+unit/FunctionTests.m +++ b/+tests/+unit/FunctionTests.m @@ -1,6 +1,20 @@ classdef FunctionTests < matlab.unittest.TestCase % FunctionTests - Unit test for functions. + 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 testString2ValidName(testCase) testCase.verifyWarning( ... From 63c2194355ab835a19937bab50ab9ae80a1a5280 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Thu, 30 Jan 2025 19:28:05 +0100 Subject: [PATCH 42/47] add test-requirement --- +tests/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/+tests/requirements.txt b/+tests/requirements.txt index c0038019c..09a7647d0 100644 --- a/+tests/requirements.txt +++ b/+tests/requirements.txt @@ -1,3 +1,4 @@ hdf5plugin +scipy git+https://github.com/NeurodataWithoutBorders/nwbinspector.git@dev git+https://github.com/NeurodataWithoutBorders/pynwb.git@dev From 29bf43278525917c5f12e88f28ed6c12077cccae Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 31 Jan 2025 15:37:52 +0100 Subject: [PATCH 43/47] Fix: Ensure object is group before deleting --- +io/+internal/+h5/deleteGroup.m | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/+io/+internal/+h5/deleteGroup.m b/+io/+internal/+h5/deleteGroup.m index 6755b3116..290ecf472 100644 --- a/+io/+internal/+h5/deleteGroup.m +++ b/+io/+internal/+h5/deleteGroup.m @@ -22,7 +22,16 @@ function deleteGroup(fileReference, groupLocation) % Open the HDF5 file in read-write mode [fileId, fileCleanupObj] = io.internal.h5.resolveFileReference(fileReference, "w"); %#ok + + [objectId, objectCleanupObj] = io.internal.h5.openObject(fileId, groupLocation); %#ok + objInfo = H5O.get_info(objectId); + clear objectCleanupObj - % Delete the group - H5L.delete(fileId, groupLocation, 'H5P_DEFAULT'); + if objInfo.type == H5ML.get_constant_value('H5O_TYPE_GROUP') + % Delete the group + H5L.delete(fileId, groupLocation, 'H5P_DEFAULT'); + else + error('NWB:DeleteGroup:NotAGroup', ... + 'The h5 object in location "%s" is not a group', groupLocation) + end end From 680c4cf014f1a2b9fea9baca817f0f78f0a29c58 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 31 Jan 2025 15:38:17 +0100 Subject: [PATCH 44/47] Fix error id --- +io/+internal/+h5/mustBeH5File.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/+io/+internal/+h5/mustBeH5File.m b/+io/+internal/+h5/mustBeH5File.m index 25794de97..78ca736a8 100644 --- a/+io/+internal/+h5/mustBeH5File.m +++ b/+io/+internal/+h5/mustBeH5File.m @@ -10,7 +10,7 @@ function mustBeH5File(value) if ~hasH5Extension exception = MException(... - 'MatNWB:validators:mustBeH5File', ... + 'NWB:validators:mustBeH5File', ... 'Expected file "%s" to have .h5 or .nwb file extension', value); throwAsCaller(exception) end From 2a3c86205222504613bdaa6173280a0bd1feaf83 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 31 Jan 2025 15:38:56 +0100 Subject: [PATCH 45/47] Add unittests for functions in io.internal.h5 namespace --- .../+io/+internal/+h5/DeleteAttributeTest.m | 109 ++++++++++++++ .../+unit/+io/+internal/+h5/DeleteGroupTest.m | 85 +++++++++++ .../+io/+internal/+h5/ListGroupNamesTest.m | 73 +++++++++ .../+io/+internal/+h5/MustBeH5FileTest.m | 90 ++++++++++++ +tests/+unit/+io/+internal/+h5/OpenFileTest.m | 111 ++++++++++++++ .../+unit/+io/+internal/+h5/OpenGroupTest.m | 139 ++++++++++++++++++ .../+unit/+io/+internal/+h5/OpenObjectTest.m | 136 +++++++++++++++++ .../+internal/+h5/ResolveFileReferenceTest.m | 118 +++++++++++++++ .../+io/+internal/+h5/ValidateLocationTest.m | 20 +++ 9 files changed, 881 insertions(+) create mode 100644 +tests/+unit/+io/+internal/+h5/DeleteAttributeTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/DeleteGroupTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/ListGroupNamesTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/MustBeH5FileTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/OpenFileTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/OpenGroupTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/OpenObjectTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/ResolveFileReferenceTest.m create mode 100644 +tests/+unit/+io/+internal/+h5/ValidateLocationTest.m diff --git a/+tests/+unit/+io/+internal/+h5/DeleteAttributeTest.m b/+tests/+unit/+io/+internal/+h5/DeleteAttributeTest.m new file mode 100644 index 000000000..a2d615e7f --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/DeleteAttributeTest.m @@ -0,0 +1,109 @@ +classdef DeleteAttributeTest < matlab.unittest.TestCase + properties + TestFile + end + + properties (Constant) + GroupLocation = "/test_group" + GroupAttributName = ["group_attr1", "group_attr2"] + DatasetLocation = "/test_dataset" + DatasetAttributName = ["dataset_attr1", "dataset_attr2"] + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file with test objects and attributes + testCase.TestFile = [tempname '.h5']; + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create a group with attributes + groupId = H5G.create(fileId, '/test_group', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5A.create(groupId, 'group_attr1', 'H5T_NATIVE_INT', H5S.create('H5S_SCALAR'), 'H5P_DEFAULT'); + H5A.create(groupId, 'group_attr2', 'H5T_NATIVE_INT', H5S.create('H5S_SCALAR'), 'H5P_DEFAULT'); + H5G.close(groupId); + + % Create a dataset with attributes + space_id = H5S.create_simple(1, 1, []); + dset_id = H5D.create(fileId, '/test_dataset', 'H5T_NATIVE_INT', space_id, 'H5P_DEFAULT'); + H5A.create(dset_id, 'dataset_attr1', 'H5T_NATIVE_INT', H5S.create('H5S_SCALAR'), 'H5P_DEFAULT'); + H5A.create(dset_id, 'dataset_attr2', 'H5T_NATIVE_INT', H5S.create('H5S_SCALAR'), 'H5P_DEFAULT'); + H5D.close(dset_id); + H5S.close(space_id); + + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testDeleteGroupAttribute(testCase) + % Test deleting an attribute from a group + + groupLocation = "/test_group"; + attributeName = ["group_attr1", "group_attr2"]; + + io.internal.h5.deleteAttribute(... + testCase.TestFile, groupLocation, attributeName(1)); + + % Verify that first attribute is deleted + testCase.verifyError(@(varargin) h5readatt(... + testCase.TestFile, groupLocation, attributeName(1)), ... + 'MATLAB:imagesci:hdf5lib:libraryError' ) + + attrValue = h5readatt(... + testCase.TestFile, groupLocation, attributeName(2)); + + % Verify that second attribute still exists + testCase.verifyEqual(attrValue, int32(0)) + end + + function testDeleteDatasetAttribute(testCase) + % Test deleting an attribute from a dataset + + datasetLocation = "/test_dataset"; + datasetAttributName = ["dataset_attr1", "dataset_attr2"]; + + io.internal.h5.deleteAttribute(... + testCase.TestFile, datasetLocation, datasetAttributName(1)); + + % Verify that first attribute is deleted + testCase.verifyError( ... + @(varargin) h5readatt(... + testCase.TestFile, datasetLocation, datasetAttributName(1)), ... + 'MATLAB:imagesci:hdf5lib:libraryError' ) + + attrValue = h5readatt(... + testCase.TestFile, datasetLocation, datasetAttributName(2)); + + % Verify that second attribute still exists + testCase.verifyEqual(attrValue, int32(0)) + end + + function testDeleteNonexistentAttribute(testCase) + % Test deleting a nonexistent attribute + testCase.verifyError(... + @() io.internal.h5.deleteAttribute( ... + testCase.TestFile, "/test_group", "nonexistent_attr"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + end + + function testDeleteFromNonexistentObject(testCase) + % Test deleting attribute from nonexistent object + testCase.verifyError(... + @() io.internal.h5.deleteAttribute( ... + testCase.TestFile, "/nonexistent", "attr"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + end + + function testDeleteWithInvalidFile(testCase) + % Test deleting from a nonexistent file + nonexistentFile = 'nonexistent.h5'; + testCase.verifyError(... + @() io.internal.h5.deleteAttribute( ... + nonexistentFile, "/test_group", "group_attr1"), ... + 'MATLAB:validators:mustBeFile'); + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/DeleteGroupTest.m b/+tests/+unit/+io/+internal/+h5/DeleteGroupTest.m new file mode 100644 index 000000000..cc2666ec8 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/DeleteGroupTest.m @@ -0,0 +1,85 @@ +classdef DeleteGroupTest < matlab.unittest.TestCase + properties + TestFile + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file with test groups + testCase.TestFile = [tempname '.h5']; + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create test groups + H5G.create(fileId, '/group1', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + group2Id = H5G.create(fileId, '/group2', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.create(group2Id, 'subgroup', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.close(group2Id); + + H5G.create(fileId, '/group3', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create a dataset (to test error case) + space_id = H5S.create_simple(1, 1, []); + dset_id = H5D.create(fileId, '/dataset1', 'H5T_NATIVE_INT', space_id, 'H5P_DEFAULT'); + H5D.close(dset_id); + H5S.close(space_id); + + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testDeleteTopLevelGroup(testCase) + % Test deleting a top-level group + io.internal.h5.deleteGroup(testCase.TestFile, "/group1"); + + testCase.verifyError(... + @(varargin) h5info(testCase.TestFile, "/group1"), ... + 'MATLAB:imagesci:h5info:unableToFind') + end + + function testDeleteNestedGroup(testCase) + % Test deleting a nested group + io.internal.h5.deleteGroup(testCase.TestFile, "/group2/subgroup"); + + testCase.verifyError(... + @(varargin) h5info(testCase.TestFile, "/group2/subgroup"), ... + 'MATLAB:imagesci:h5info:unableToFind') + + S = h5info(testCase.TestFile, "/group2"); + testCase.verifyClass(S, 'struct'); + testCase.verifyEqual(S.Name, '/group2'); + end + + function testDeleteNonexistentGroup(testCase) + % Test deleting a nonexistent group + testCase.verifyError(... + @() io.internal.h5.deleteGroup(testCase.TestFile, "/nonexistent"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + end + + function testDeleteDataset(testCase) + % Test deleting a dataset using deleteGroup (should fail) + testCase.verifyError(... + @() io.internal.h5.deleteGroup(testCase.TestFile, "/dataset1"), ... + 'NWB:DeleteGroup:NotAGroup'); + end + + function testDeleteWithRelativePath(testCase) + % Test deleting using a relative path + io.internal.h5.deleteGroup(testCase.TestFile, "group3"); + + % Verify group is deleted + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); %#ok + + testCase.verifyFalse(... + logical(H5L.exists(fileId, '/group3', 'H5P_DEFAULT')), ... + 'Group should be deleted when using relative path'); + + clear fileCleanup; + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/ListGroupNamesTest.m b/+tests/+unit/+io/+internal/+h5/ListGroupNamesTest.m new file mode 100644 index 000000000..e1e7e3a2f --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/ListGroupNamesTest.m @@ -0,0 +1,73 @@ +classdef ListGroupNamesTest < matlab.unittest.TestCase + properties + TestFile + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file + testCase.TestFile = [tempname '.h5']; + + % Create test file with groups + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create root level groups + group1Id = H5G.create(fileId, '/group1', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + group2Id = H5G.create(fileId, '/group2', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create nested groups + H5G.create(group1Id, 'subgroup1', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.create(group1Id, 'subgroup2', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create a dataset (should not be listed as a group) + space_id = H5S.create_simple(1, 1, []); + dset_id = H5D.create(fileId, '/dataset1', 'H5T_NATIVE_INT', space_id, 'H5P_DEFAULT'); + + % Close everything + H5D.close(dset_id); + H5S.close(space_id); + H5G.close(group1Id); + H5G.close(group2Id); + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testRootLevelGroups(testCase) + % Test listing groups at root level + groups = io.internal.h5.listGroupNames(testCase.TestFile, "/"); + testCase.verifyEqual(sort(groups), sort({'group1', 'group2'}), ... + 'Root level groups not listed correctly'); + end + + function testNestedGroups(testCase) + % Test listing nested groups + groups = io.internal.h5.listGroupNames(testCase.TestFile, "/group1"); + testCase.verifyEqual(sort(groups), sort({'subgroup1', 'subgroup2'}), ... + 'Nested groups not listed correctly'); + end + + function testEmptyGroup(testCase) + % Test listing groups in an empty group + groups = io.internal.h5.listGroupNames(testCase.TestFile, "/group2"); + testCase.verifyEmpty(groups, 'Empty group should return empty cell array'); + end + + function testNonexistentLocation(testCase) + % Test error when location doesn't exist + testCase.verifyError(... + @() io.internal.h5.listGroupNames(testCase.TestFile, "/nonexistent"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + end + + function testDatasetLocation(testCase) + % Test error when location is a dataset + testCase.verifyError(... + @() io.internal.h5.listGroupNames(testCase.TestFile, "/dataset1"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/MustBeH5FileTest.m b/+tests/+unit/+io/+internal/+h5/MustBeH5FileTest.m new file mode 100644 index 000000000..be6606824 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/MustBeH5FileTest.m @@ -0,0 +1,90 @@ +classdef MustBeH5FileTest < matlab.unittest.TestCase + + properties (TestParameter) + ValidFileName = {'test_file.h5', 'test_file.nwb'} + InvalidFileName = {'test_file.txt'} + end + + properties (Constant) + CreateFileFunction = containers.Map(... + {'.nwb', '.h5'}, {'createNwbFile','createH5File'} ) + end + + methods (TestClassSetup) + function createTestFiles(testCase) + + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + + % Create temporary test files + h5FileName = testCase.ValidFileName{contains(testCase.ValidFileName, '.h5')}; + nwbFileName = testCase.ValidFileName{contains(testCase.ValidFileName, '.nwb')}; + + testCase.createH5File(h5FileName) + testCase.createNwbFile(nwbFileName) + + % Create file which is not h5 + system( sprintf("touch %s", testCase.InvalidFileName{1}) ) + end + end + + methods (Test) + function testValidCharPath(testCase, ValidFileName) %#ok + io.internal.h5.mustBeH5File( char(ValidFileName) ) + io.internal.h5.mustBeH5FileReference( char(ValidFileName) ) + end + + function testValidStringPath(testCase, ValidFileName) %#ok + io.internal.h5.mustBeH5FileReference( string(ValidFileName) ) + end + + function testValidH5MLId(testCase, ValidFileName) %#ok + [fileId, fileCleanup] = io.internal.h5.openFile(ValidFileName); %#ok + io.internal.h5.mustBeH5FileReference(fileId) + end + + function testInvalidFileExtension(testCase, InvalidFileName) + testCase.verifyError(... + @() io.internal.h5.mustBeH5FileReference(InvalidFileName), ... + 'NWB:validators:mustBeH5File'); + end + + function testNonexistentFile(testCase) + nonexistentFile = 'nonexistent.h5'; + testCase.verifyError(... + @() io.internal.h5.mustBeH5FileReference(nonexistentFile), ... + 'MATLAB:validators:mustBeFile'); + end + + function testInvalidInputType(testCase) + % Test with invalid input type + testCase.verifyError(... + @() io.internal.h5.mustBeH5FileReference(123), ... + 'MATLAB:validators:mustBeA'); + end + + function testEmptyString(testCase) + % Test with empty string + testCase.verifyError(... + @() io.internal.h5.mustBeH5FileReference(""), ... + 'MATLAB:validators:mustBeNonzeroLengthText'); + end + end + + methods (Static) + function createH5File(filePath) + fileId = H5F.create(filePath, ... + 'H5F_ACC_TRUNC', ... + 'H5P_DEFAULT', ... + 'H5P_DEFAULT'); + H5F.close(fileId); + end + + function createNwbFile(filePath) + nwbFile = NwbFile( ... + 'session_description', 'Test file for nwb export', ... + 'identifier', 'export_test', ... + 'session_start_time', datetime("now", 'TimeZone', 'local') ); + nwbExport(nwbFile, filePath) + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/OpenFileTest.m b/+tests/+unit/+io/+internal/+h5/OpenFileTest.m new file mode 100644 index 000000000..430efc320 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/OpenFileTest.m @@ -0,0 +1,111 @@ +classdef OpenFileTest < matlab.unittest.TestCase + properties + TestFile + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file + testCase.TestFile = [tempname '.h5']; + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testDefaultReadOnlyAccess(testCase) + % Test default read-only access + [fileId, cleanupObj] = io.internal.h5.openFile(testCase.TestFile); %#ok<*ASGLU> + testCase.verifyTrue( logical(H5F.is_hdf5(testCase.TestFile)) ); + + % Verify read-only by attempting to create a group (should fail) + testCase.verifyError(@() H5G.create(fileId, '/test_group', ... + 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + % Clear cleanup object explicitly to close file + clear cleanupObj; + end + + function testExplicitReadOnlyAccess(testCase) + % Test explicit read-only access + [fileId, cleanupObj] = io.internal.h5.openFile(testCase.TestFile, "r"); + testCase.verifyTrue( logical(H5F.is_hdf5(testCase.TestFile)) ); + + % Verify read-only by attempting to create a group (should fail) + testCase.verifyError(@() H5G.create(fileId, '/test_group', ... + 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + clear cleanupObj; + end + + function testWriteAccess(testCase) + % Test write access + [fileId, cleanupObj] = io.internal.h5.openFile(testCase.TestFile, "w"); + testCase.verifyTrue( logical(H5F.is_hdf5(testCase.TestFile)) ); + + % Verify write access by creating a group + testCase.verifyWarningFree(@() H5G.create(fileId, '/test_group', ... + 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT')); + + clear cleanupObj; + end + + function testFileCleanup(testCase) + % Test that cleanup object properly closes file + [fileId, cleanupObj] = io.internal.h5.openFile(testCase.TestFile); + + % Clear cleanup object to close file + clear cleanupObj; + + testCase.assertClass(fileId, "H5ML.id") + testCase.assertEqual(double(fileId), -1) + end + + + function testInvalidPermission(testCase) + % Test invalid permission argument + testCase.verifyError(... + @() io.internal.h5.openFile(testCase.TestFile, "x"), ... + 'MATLAB:validators:mustBeMember'); + end + + function testMultipleOpens(testCase) + % Test opening file multiple times + [fileId1, cleanupObj1] = io.internal.h5.openFile(testCase.TestFile); + [fileId2, cleanupObj2] = io.internal.h5.openFile(testCase.TestFile); + + % Verify both file IDs are valid and different + testCase.verifyNotEqual(fileId1, fileId2, ... + 'Multiple opens should return different file IDs'); + + clear cleanupObj1 cleanupObj2; + end + + function testReadWriteOperations(testCase) + % Test combined read/write operations + % First write some data + [fileId, cleanupObj] = io.internal.h5.openFile(testCase.TestFile, "w"); + + % Create a group + groupId = H5G.create(fileId, '/test_data', 'H5P_DEFAULT', ... + 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.close(groupId); + + clear cleanupObj; + + % Then read it back + [fileId, cleanupObj] = io.internal.h5.openFile(testCase.TestFile, "r"); + + % Verify group exists + existGroup = H5L.exists(fileId, '/test_data', 'H5P_DEFAULT'); + testCase.verifyTrue(logical(existGroup)); + + clear cleanupObj; + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/OpenGroupTest.m b/+tests/+unit/+io/+internal/+h5/OpenGroupTest.m new file mode 100644 index 000000000..b03ba35b4 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/OpenGroupTest.m @@ -0,0 +1,139 @@ +classdef OpenGroupTest < matlab.unittest.TestCase + properties + TestFile + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file with test groups + testCase.TestFile = [tempname '.h5']; + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create test groups + H5G.create(fileId, '/group1', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + group2Id = H5G.create(fileId, '/group2', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.create(group2Id, 'subgroup', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.close(group2Id); + + % Create a dataset (to test error case) + space_id = H5S.create_simple(1, 1, []); + dset_id = H5D.create(fileId, '/dataset1', 'H5T_NATIVE_INT', space_id, 'H5P_DEFAULT'); + H5D.close(dset_id); + H5S.close(space_id); + + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testOpenRootGroup(testCase) + % Test opening root group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); %#ok<*ASGLU> + [groupId, groupCleanup] = io.internal.h5.openGroup(fileId, "/"); + + % Verify group is valid + testCase.verifyTrue(logical(H5I.is_valid(groupId))); + + clear groupCleanup fileCleanup; + end + + function testOpenTopLevelGroup(testCase) + % Test opening a top-level group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + [groupId, groupCleanup] = io.internal.h5.openGroup(fileId, "/group1"); + + % Verify group is valid + testCase.verifyTrue(logical(H5I.is_valid(groupId))); + + clear groupCleanup fileCleanup; + end + + function testOpenNestedGroup(testCase) + % Test opening a nested group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + [groupId, groupCleanup] = io.internal.h5.openGroup(fileId, "/group2/subgroup"); + + % Verify group is valid + testCase.verifyTrue(logical(H5I.is_valid(groupId))); + + clear groupCleanup fileCleanup; + end + + function testGroupCleanup(testCase) + % Test that cleanup object properly closes group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + % Get group ID before cleanup + [groupId, groupCleanup] = io.internal.h5.openGroup(fileId, "/group1"); + originalId = groupId; + + % Clear cleanup object to close group + clear groupCleanup; + + % Verify group is no longer valid + testCase.verifyFalse(logical(H5I.is_valid(originalId))); + + + testCase.assertClass(originalId, "H5ML.id") + testCase.assertEqual(double(originalId), -1) + + clear fileCleanup; + end + + function testNonexistentGroup(testCase) + % Test opening nonexistent group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + testCase.verifyError(... + @() io.internal.h5.openGroup(fileId, "/nonexistent"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + clear fileCleanup; + end + + function testOpenDatasetAsGroup(testCase) + % Test attempting to open a dataset as a group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + testCase.verifyError(... + @() io.internal.h5.openGroup(fileId, "/dataset1"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + clear fileCleanup; + end + + function testMultipleOpens(testCase) + % Test opening same group multiple times + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + [groupId1, groupCleanup1] = io.internal.h5.openGroup(fileId, "/group1"); + [groupId2, groupCleanup2] = io.internal.h5.openGroup(fileId, "/group1"); + + % Verify both group IDs are valid and different + testCase.verifyTrue(logical(H5I.is_valid(groupId1))); + testCase.verifyTrue(logical(H5I.is_valid(groupId2))); + testCase.verifyNotEqual(groupId1, groupId2); + + clear groupCleanup1 groupCleanup2 fileCleanup; + end + + function testLocationNormalization(testCase) + % Test that different path formats work + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + % Test various path formats + paths = {"/group1", "group1", "/group1/", "//group1", "group1/"}; + + for i = 1:length(paths) + [groupId, groupCleanup] = io.internal.h5.openGroup(fileId, paths{i}); + testCase.verifyTrue(logical(H5I.is_valid(groupId))); + clear groupCleanup; + end + + clear fileCleanup; + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/OpenObjectTest.m b/+tests/+unit/+io/+internal/+h5/OpenObjectTest.m new file mode 100644 index 000000000..ce0909277 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/OpenObjectTest.m @@ -0,0 +1,136 @@ +classdef OpenObjectTest < matlab.unittest.TestCase + properties + TestFile + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file with test objects + testCase.TestFile = [tempname '.h5']; + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create test group + H5G.create(fileId, '/test_group', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + + % Create nested group + nestedGroupId = H5G.create(fileId, '/test_group/nested_group', 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5G.close(nestedGroupId); + + % Create test dataset + space_id = H5S.create_simple(1, 1, []); + dset_id = H5D.create(fileId, '/test_dataset', 'H5T_NATIVE_INT', space_id, 'H5P_DEFAULT'); + H5D.close(dset_id); + H5S.close(space_id); + + % Create dataset in group + space_id = H5S.create_simple(1, 1, []); + dset_id = H5D.create(fileId, '/test_group/group_dataset', 'H5T_NATIVE_INT', space_id, 'H5P_DEFAULT'); + H5D.close(dset_id); + H5S.close(space_id); + + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testOpenGroup(testCase) + % Test opening a group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); %#ok<*ASGLU> + [objectId, objectCleanup] = io.internal.h5.openObject(fileId, "/test_group"); + + % Verify object is valid and is a group + testCase.verifyTrue(logical(H5I.is_valid(objectId))); + objInfo = H5O.get_info(objectId); + testCase.verifyEqual(objInfo.type, H5ML.get_constant_value('H5O_TYPE_GROUP')); + + clear objectCleanup fileCleanup; + end + + function testOpenDataset(testCase) + % Test opening a dataset + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + [objectId, objectCleanup] = io.internal.h5.openObject(fileId, "/test_dataset"); + + % Verify object is valid and is a dataset + testCase.verifyTrue(logical(H5I.is_valid(objectId))); + objInfo = H5O.get_info(objectId); + testCase.verifyEqual(objInfo.type, H5ML.get_constant_value('H5O_TYPE_DATASET')); + + clear objectCleanup fileCleanup; + end + + function testOpenNestedGroup(testCase) + % Test opening a nested group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + [objectId, objectCleanup] = io.internal.h5.openObject(fileId, "/test_group/nested_group"); + + % Verify object is valid and is a group + testCase.verifyTrue(logical(H5I.is_valid(objectId))); + objInfo = H5O.get_info(objectId); + testCase.verifyEqual(objInfo.type, H5ML.get_constant_value('H5O_TYPE_GROUP')); + + clear objectCleanup fileCleanup; + end + + function testOpenDatasetInGroup(testCase) + % Test opening a dataset inside a group + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + [objectId, objectCleanup] = io.internal.h5.openObject(fileId, "/test_group/group_dataset"); + + % Verify object is valid and is a dataset + testCase.verifyTrue(logical(H5I.is_valid(objectId))); + objInfo = H5O.get_info(objectId); + testCase.verifyEqual(objInfo.type, H5ML.get_constant_value('H5O_TYPE_DATASET')); + + clear objectCleanup fileCleanup; + end + + function testObjectCleanup(testCase) + % Test that cleanup object properly closes object + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + % Get object ID before cleanup + [objectId, objectCleanup] = io.internal.h5.openObject(fileId, "/test_group"); + originalId = objectId; + + % Clear cleanup object to close object + clear objectCleanup; + + % Verify object is no longer valid + testCase.verifyFalse(logical(H5I.is_valid(originalId))); + testCase.assertClass(originalId, "H5ML.id") + testCase.assertEqual(double(originalId), -1) + + clear fileCleanup; + end + + function testOpenNonexistentObject(testCase) + % Test opening nonexistent object + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + testCase.verifyError(... + @() io.internal.h5.openObject(fileId, "/nonexistent"), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + clear fileCleanup; + end + + function testMultipleOpens(testCase) + % Test opening same object multiple times + [fileId, fileCleanup] = io.internal.h5.openFile(testCase.TestFile); + + [objectId1, objectCleanup1] = io.internal.h5.openObject(fileId, "/test_group"); + [objectId2, objectCleanup2] = io.internal.h5.openObject(fileId, "/test_group"); + + % Verify both object IDs are valid and different + testCase.verifyTrue(logical(H5I.is_valid(objectId1))); + testCase.verifyTrue(logical(H5I.is_valid(objectId2))); + testCase.verifyNotEqual(objectId1, objectId2); + + clear objectCleanup1 objectCleanup2 fileCleanup; + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/ResolveFileReferenceTest.m b/+tests/+unit/+io/+internal/+h5/ResolveFileReferenceTest.m new file mode 100644 index 000000000..83e4f8352 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/ResolveFileReferenceTest.m @@ -0,0 +1,118 @@ +classdef ResolveFileReferenceTest < matlab.unittest.TestCase + properties + TestFile + end + + methods (TestClassSetup) + function createTestFile(testCase) + % Create a temporary test file + testCase.TestFile = [tempname '.h5']; + fileId = H5F.create(testCase.TestFile, 'H5F_ACC_TRUNC', 'H5P_DEFAULT', 'H5P_DEFAULT'); + H5F.close(fileId); + + % Add test file cleanup + testCase.addTeardown(@delete, testCase.TestFile); + end + end + + methods (Test) + function testResolveFilePath(testCase) + % Test resolving a file path + [fileId, cleanupObj] = io.internal.h5.resolveFileReference(testCase.TestFile); + + % Verify file ID is valid + testCase.verifyTrue(logical(H5I.is_valid(fileId))); + % Verify cleanup object is created + testCase.verifyNotEmpty(cleanupObj); + + clear cleanupObj; + end + + function testResolveFileId(testCase) + % Test resolving an existing file ID + [originalId, originalCleanup] = io.internal.h5.openFile(testCase.TestFile); %#ok<*ASGLU> + + % Resolve the file ID + [resolvedId, cleanupObj] = io.internal.h5.resolveFileReference(originalId); + + % Verify resolved ID matches original + testCase.verifyEqual(resolvedId, originalId); + % Verify no cleanup object is created + testCase.verifyEmpty(cleanupObj); + + clear originalCleanup; + end + + function testReadPermission(testCase) + % Test resolving with read permission + [fileId, cleanupObj] = io.internal.h5.resolveFileReference(testCase.TestFile, "r"); + + % Verify read-only by attempting to create a group (should fail) + testCase.verifyError(@() H5G.create(fileId, '/test_group', ... + 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + clear cleanupObj; + end + + function testWritePermission(testCase) + % Test resolving with write permission + [fileId, cleanupObj] = io.internal.h5.resolveFileReference(testCase.TestFile, "w"); + + % Verify write access by creating a group + testCase.verifyWarningFree(@() H5G.create(fileId, '/test_group', ... + 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT')); + + clear cleanupObj; + end + + function testDefaultPermission(testCase) + % Test default permission (should be read) + [fileId, cleanupObj] = io.internal.h5.resolveFileReference(testCase.TestFile); + + % Verify read-only by attempting to create a group (should fail) + testCase.verifyError(@() H5G.create(fileId, '/test_group', ... + 'H5P_DEFAULT', 'H5P_DEFAULT', 'H5P_DEFAULT'), ... + 'MATLAB:imagesci:hdf5lib:libraryError'); + + clear cleanupObj; + end + + function testNonexistentFile(testCase) + % Test resolving nonexistent file + nonexistentFile = 'nonexistent.h5'; + testCase.verifyError(... + @() io.internal.h5.resolveFileReference(nonexistentFile), ... + 'MATLAB:validators:mustBeFile'); + end + + function testInvalidPermission(testCase) + % Test invalid permission argument + testCase.verifyError(... + @() io.internal.h5.resolveFileReference(testCase.TestFile, "x"), ... + 'MATLAB:validators:mustBeMember'); + end + + function testNoCleanupWithFileId(testCase) + % Test that no cleanup occurs when passing file ID + [originalId, originalCleanup] = io.internal.h5.openFile(testCase.TestFile); + + % Resolve multiple times + [resolvedId1, cleanup1] = io.internal.h5.resolveFileReference(originalId); + [resolvedId2, cleanup2] = io.internal.h5.resolveFileReference(originalId); + + % Verify all IDs are the same + testCase.verifyEqual(resolvedId1, originalId); + testCase.verifyEqual(resolvedId2, originalId); + + % Verify no cleanup objects were created + testCase.verifyEmpty(cleanup1); + testCase.verifyEmpty(cleanup2); + + % Verify ID is still valid + testCase.verifyTrue(logical(H5I.is_valid(originalId))); + + clear originalCleanup; + end + end +end diff --git a/+tests/+unit/+io/+internal/+h5/ValidateLocationTest.m b/+tests/+unit/+io/+internal/+h5/ValidateLocationTest.m new file mode 100644 index 000000000..6756eb716 --- /dev/null +++ b/+tests/+unit/+io/+internal/+h5/ValidateLocationTest.m @@ -0,0 +1,20 @@ +classdef ValidateLocationTest < matlab.unittest.TestCase + methods (Test) + function testAbsolutePath(testCase) + % Test path that already starts with / + input = "/path/to/group"; + result = io.internal.h5.validateLocation(input); + testCase.verifyEqual(result, input, ... + 'Absolute path should remain unchanged'); + end + + function testRelativePath(testCase) + % Test path without leading / + input = "path/to/group"; + expected = "/path/to/group"; + result = io.internal.h5.validateLocation(input); + testCase.verifyEqual(result, expected, ... + 'Relative path should be converted to absolute'); + end + end +end From 4d7c5b7f7afb82a79e60ee2b948230d118d97ef9 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Mon, 3 Feb 2025 20:35:48 +0100 Subject: [PATCH 46/47] Update nwbExportTest.m Added comments and a better test to test for warning with ID 'NWB:validators:MissingEmbeddedNamespace' --- +tests/+unit/nwbExportTest.m | 45 ++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/+tests/+unit/nwbExportTest.m b/+tests/+unit/nwbExportTest.m index 0590d17e3..26128c1f8 100644 --- a/+tests/+unit/nwbExportTest.m +++ b/+tests/+unit/nwbExportTest.m @@ -89,7 +89,7 @@ function testEmbeddedSpecs(testCase) testCase.verifyEmpty(embeddedNamespaces) ts = types.core.TimeSeries('data', rand(1,10), 'timestamps', 1:10); - nwb.acquisition.set('test', ts) + nwb.acquisition.set('test', ts); nwbExport(nwb, nwbFileName); embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); @@ -107,17 +107,52 @@ function testEmbeddedSpecs(testCase) % Verify that extension namespace is part of embedded specs. testCase.verifyEqual(sort(embeddedNamespaces), {'core', 'hdmf-common', 'ndx-photostim'}) + % When we remove the TestDevice from the NWB object, and + % re-export, the ndx-photostim namespace/extension should be + % removed from the embedded specifications in the file, because + % there are not longer any types from the ndx-photostim + % extension in the file. + + % Please note: The following commands only removes the + % TestDevice from the nwbFile object, not from the actual file + % See matnwb issue #649: + % https://github.com/NeurodataWithoutBorders/matnwb/issues/649 nwb.general_devices.remove('TestDevice'); nwbExport(nwb, nwbFileName); embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); testCase.verifyEqual(sort(embeddedNamespaces), {'core', 'hdmf-common'}) + end + + function testWarnIfMissingNamespaceSpecification(testCase) + % Tests the case where a cached namespace specification is + % deleted from disk before an nwb object containing types from + % that namespace is exported to file. + + nwbFileName = 'testWarnIfMissingNamespaceSpecification.nwb'; + % Install extension. + nwbInstallExtension("ndx-photostim", 'savedir', '.') + + % Export a file not using a type from an extension + nwb = testCase.initNwbFile(); + + % Add a timeseries object + ts = types.core.TimeSeries('data', rand(1,10), 'timestamps', 1:10); + nwb.acquisition.set('test', ts); + + % Add type from ndx-photostim extension. + testDevice = types.ndx_photostim.Laser('model', 'Spectra-Physics'); + nwb.general_devices.set('TestDevice', testDevice); + + % Simulate the rare case where a user might delete the cached + % namespace specification before exporting a file + cachedNamespaceSpec = fullfile("namespaces/ndx-photostim.mat"); + delete(cachedNamespaceSpec) + % Test that warning for missing namespace works - [fileId, fileCleanupObj] = io.internal.h5.openFile(nwbFileName); %#ok - expectedNamespaces = {'core', 'hdmf-common', 'ndx-photostim'}; - testCase.verifyWarning( ... - @(fid,names) io.spec.validateEmbeddedSpecifications(fileId, expectedNamespaces), ... + testCase.verifyWarning(... + @() nwbExport(nwb, nwbFileName), ... 'NWB:validators:MissingEmbeddedNamespace') end end From 40f5de36fd66f0382a8566d7d0f7a4429233adb0 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Tue, 4 Feb 2025 14:37:45 +0100 Subject: [PATCH 47/47] Fix failing tests --- +tests/+unit/nwbExportTest.m | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/+tests/+unit/nwbExportTest.m b/+tests/+unit/nwbExportTest.m index 91af146d4..65130646f 100644 --- a/+tests/+unit/nwbExportTest.m +++ b/+tests/+unit/nwbExportTest.m @@ -59,7 +59,7 @@ function testExportNwbFileWithMissingRequiredLink(testCase) % an error. electrode = types.core.IntracellularElectrode('description', 'test'); - testCase.NwbObject.general_intracellular_ephys.set('Electrode', electrode) + testCase.NwbObject.general_intracellular_ephys.set('Electrode', electrode); nwbFilePath = 'testExportNwbFileWithMissingRequiredLink.nwb'; testCase.verifyError(@(f, fn) nwbExport(testCase.NwbObject, nwbFilePath), ... @@ -141,7 +141,8 @@ function testEmbeddedSpecs(testCase) embeddedNamespaces = io.spec.listEmbeddedSpecNamespaces(nwbFileName); testCase.verifyEmpty(embeddedNamespaces) - ts = types.core.TimeSeries('data', rand(1,10), 'timestamps', 1:10); + ts = types.core.TimeSeries(... + 'data', rand(1,10), 'timestamps', 1:10, 'data_unit', 'test'); nwb.acquisition.set('test', ts); nwbExport(nwb, nwbFileName); @@ -191,7 +192,8 @@ function testWarnIfMissingNamespaceSpecification(testCase) nwb = testCase.initNwbFile(); % Add a timeseries object - ts = types.core.TimeSeries('data', rand(1,10), 'timestamps', 1:10); + ts = types.core.TimeSeries(... + 'data', rand(1,10), 'timestamps', 1:10, 'data_unit', 'test'); nwb.acquisition.set('test', ts); % Add type from ndx-photostim extension.