Refactor: Add arguments block to main MatNWB api functions (#619)

* Refactor: Add arguments block to main MatNWB api functions - Move some functions io.spec namespace - Add validation functions in matnwb.common namespace * Update functionSignatures * Create functionSignatures.json * Improve coverage for main functions * Create TypeConversionTest.m * Add unit tests for io.isBool and io.pathParts * Update functionSignatures * Add test for cloneNwbFileClass * Create local validator function for reftype in file.fillProps - Simplify logic in fillExport * Create local function for duplicated code block * Simplify file cleanup in case of error * Update writeNamespace.m continue is unecessary here * Fix failing tests * Improve coverage * Add tests for misc functions * Simplify spec.loadCache Add arguments block * Remove unused function * Add misc unittests * Add datapipe test and fix bug in DynamicFilter class * Add unittests for functions in +types namespace * Add class setup to +types function tests * Add tests for clearing dynamictable plus fix related bugs * Add input options for nwbClearGenerated * Add cleanup for writeAttribute Removes lines that can not be reached * Update Point.m Simplify * Add SpaceTest * Fix bug with writing and parsing logical data in compound data type * Fix variableName in test * Add more unittests to WriteTest * Remove unused function * Add unit tests for functions in +types namespace * Change exist to isfolder/isfile --------- Co-authored-by: Ben Dichter <[email protected]>
NeurodataWithoutBorders · Nov 19, 2024 · fa04ab0 · fa04ab0
1 parent 21bd143
commit fa04ab0
Show file tree

Hide file tree

Showing 53 changed files with 1,298 additions and 522 deletions.
diff --git a/+file/cloneNwbFileClass.m b/+file/cloneNwbFileClass.m
@@ -5,14 +5,21 @@ function cloneNwbFileClass(typeFileName, fullTypeName)
 
 nwbFilePath = which('NwbFile');
 installPath = fileparts(nwbFilePath);
-fileId = fopen(nwbFilePath);
-text = strrep(char(fread(fileId) .'),...
-    'NwbFile < types.core.NWBFile',...
+nwbFileClassDef = fileread(nwbFilePath);
+
+% Update superclass name
+updatedNwbFileClassDef = strrep(nwbFileClassDef, ...
+    'NwbFile < types.core.NWBFile', ...
     sprintf('NwbFile < %s', fullTypeName));
-fclose(fileId);
+
+% Update call to superclass constructor
+updatedNwbFileClassDef = strrep(updatedNwbFileClassDef, ...
+    'obj = [email protected]', ...
+    sprintf('obj = obj@%s', fullTypeName));
 
 fileId = fopen(fullfile(installPath, [typeFileName '.m']), 'W');
-fwrite(fileId, text);
+fwrite(fileId, updatedNwbFileClassDef);
 fclose(fileId);
-end
 
+rehash();
+end
diff --git a/+file/fillExport.m b/+file/fillExport.m
@@ -21,9 +21,6 @@
     for i = 1:length(propertyNames)
         propertyName = propertyNames{i};
         pathProps = traverseRaw(propertyName, RawClass);
-        if isempty(pathProps)
-            keyboard;
-        end
         prop = pathProps{end};
         elideProps = pathProps(1:end-1);
         elisions = cell(length(elideProps),1);
@@ -84,11 +81,10 @@
     path = {}; 
 
     if isa(RawClass, 'file.Dataset')
-        if isempty(RawClass.attributes)
-            return;
+        if ~isempty(RawClass.attributes)
+            matchesAttribute = strcmp({RawClass.attributes.name}, propertyName);
+            path = {RawClass.attributes(matchesAttribute)};
         end
-        matchesAttribute = strcmp({RawClass.attributes.name}, propertyName);
-        path = {RawClass.attributes(matchesAttribute)};
         return;
     end
 

diff --git a/+file/fillProps.m b/+file/fillProps.m
@@ -52,30 +52,14 @@
         typeStr = ['Table with columns: (', strjoin(columnDocStr, ', '), ')'];
     elseif isa(prop, 'file.Attribute')
         if isa(prop.dtype, 'containers.Map')
-            switch prop.dtype('reftype')
-                case 'region'
-                    refTypeName = 'Region';
-                case 'object'
-                    refTypeName = 'Object';
-                otherwise
-                    error('NWB:ClassGenerator:InvalidRefType', ...
-                        'Invalid reftype found while filling description for class property "%s".', propName);
-            end
-            typeStr = sprintf('%s Reference to %s', refTypeName, prop.dtype('target_type'));
+            assertValidRefType(prop.dtype('reftype'))
+            typeStr = sprintf('%s reference to %s', capitalize(prop.dtype('reftype')), prop.dtype('target_type'));
         else
             typeStr = prop.dtype;
         end
     elseif isa(prop, 'containers.Map')
-        switch prop('reftype')
-            case 'region'
-                refTypeName = 'region';
-            case 'object'
-                refTypeName = 'object';
-            otherwise
-                error('NWB:ClassGenerator:InvalidRefType', ...
-                    'Invalid reftype found while filling description for class property "%s".', propName);
-        end
-        typeStr = sprintf('%s Reference to %s', refTypeName, prop('target_type'));
+        assertValidRefType(prop('reftype'))
+        typeStr = sprintf('%s reference to %s', capitalize(prop('reftype')), prop('target_type'));
     elseif isa(prop, 'file.interface.HasProps')
         typeStrCell = cell(size(prop));
         for iProp = 1:length(typeStrCell)
@@ -108,4 +92,20 @@
     if nargin >= 2
         propStr = [propName ' = ' propStr];
     end
+end
+
+function assertValidRefType(referenceType)
+    arguments
+        referenceType (1,1) string
+    end
+    assert( ismember(referenceType, ["region", "object"]), ...
+        'NWB:ClassGenerator:InvalidRefType', ...
+        'Invalid reftype found while filling description for class properties.')
+end
+
+function word = capitalize(word)
+    arguments
+        word (1,:) char
+    end
+    word(1) = upper(word(1));
 end
diff --git a/+file/fillValidators.m b/+file/fillValidators.m
@@ -155,34 +155,19 @@
             fillDimensionValidation(prop.dtype, prop.shape)...
             }, newline);
     elseif prop.isConstrainedSet
-        try
-            fullname = namespaceReg.getFullClassName(prop.type);
-        catch ME
-            if ~endsWith(ME.identifier, 'Namespace:NotFound')
-                rethrow(ME);
-            end
-
-            warning('NWB:Fill:Validators:NamespaceNotFound',...
-                ['Namespace could not be found for type `%s`.' ...
-                '  Skipping Validation for property `%s`.'], prop.type, name);
-            return;
+        fullname = getFullClassName(namespaceReg, prop.type, name);
+        if isempty(fullname)
+            return
         end
+
         unitValidationStr = strjoin({unitValidationStr...
             ['constrained = { ''' fullname ''' };']...
             ['types.util.checkSet(''' name ''', struct(), constrained, val);']...
             }, newline);
     else
-        try
-            fullname = namespaceReg.getFullClassName(prop.type);
-        catch ME
-            if ~endsWith(ME.identifier, 'Namespace:NotFound')
-                rethrow(ME);
-            end
-
-            warning('NWB:Fill:Validators:NamespaceNotFound',...
-                ['Namespace could not be found for type `%s`.' ...
-                '  Skipping Validation for property `%s`.'], prop.type, name);
-            return;
+        fullname = getFullClassName(namespaceReg, prop.type, name);
+        if isempty(fullname)
+            return
         end
         unitValidationStr = [unitValidationStr newline fillDtypeValidation(name, fullname)];
     end
@@ -318,4 +303,19 @@
             condition, ...
             sprintf('    %s', errorStr), ...
             'end' }, newline );
+end
+
+function fullname = getFullClassName(namespaceReg, propType, name)
+    fullname = '';
+    try
+        fullname = namespaceReg.getFullClassName(propType);
+    catch ME
+        if ~endsWith(ME.identifier, 'Namespace:NotFound')
+            rethrow(ME);
+        end
+
+        warning('NWB:Fill:Validators:NamespaceNotFound',...
+            ['Namespace could not be found for type `%s`.' ...
+            '  Skipping Validation for property `%s`.'], propType, name);
+    end
 end
diff --git a/+file/writeNamespace.m b/+file/writeNamespace.m
@@ -4,7 +4,7 @@ function writeNamespace(namespaceName, saveDir)
 
 classFileDir = fullfile(saveDir, '+types', ['+' misc.str2validName(Namespace.name)]);
 
-if 7 ~= exist(classFileDir, 'dir')
+if ~isfolder(classFileDir)
     mkdir(classFileDir);
 end
 
@@ -14,18 +14,14 @@ function writeNamespace(namespaceName, saveDir)
     className = classes{i};
     [processed, classprops, inherited] = file.processClass(className, Namespace, pregenerated);
 
-    if isempty(processed)
-        continue;
-    end
-
-    fid = fopen(fullfile(classFileDir, [className '.m']), 'W');
-    try
+    if ~isempty(processed)
+        fid = fopen(fullfile(classFileDir, [className '.m']), 'W');
+        % Create cleanup object to close to file in case the write operation fails.
+        fileCleanupObj = onCleanup(@(id) fclose(fid));
         fwrite(fid, file.fillClass(className, Namespace, processed, ...
             classprops, inherited), 'char');
-    catch ME
-        fclose(fid);
-        rethrow(ME)
+    else
+        % pass
     end
-    fclose(fid);
 end
 end
diff --git a/+io/+space/+shape/Point.m b/+io/+space/+shape/Point.m
@@ -22,11 +22,9 @@
         end
 
         function varargout = getMatlabIndex(obj)
-            if 0 == nargout
-                return;
+            if nargout > 0
+                varargout{1} = obj.index;
             end
-
-            varargout{1} = obj.index;
         end
     end
 end

diff --git a/+io/+spec/+internal/readEmbeddedSpecLocation.m b/+io/+spec/+internal/readEmbeddedSpecLocation.m
@@ -0,0 +1,18 @@
+function specLocation = readEmbeddedSpecLocation(fid, specLocAttributeName)
+    arguments
+        fid (1,1) H5ML.id
+        specLocAttributeName (1,1) string = '.specloc'
+    end
+
+    specLocation = '';
+    try % Check .specloc
+        attributeId = H5A.open(fid, specLocAttributeName);
+        attributeCleanup = onCleanup(@(id) H5A.close(attributeId));
+        referenceRawData = H5A.read(attributeId);
+        specLocation = H5R.get_name(attributeId, 'H5R_OBJECT', referenceRawData);
+    catch ME
+        if ~strcmp(ME.identifier, 'MATLAB:imagesci:hdf5lib:libraryError')
+            rethrow(ME);
+        end % don't error if the attribute doesn't exist.
+    end
+end
diff --git a/+io/+spec/getEmbeddedSpecLocation.m b/+io/+spec/getEmbeddedSpecLocation.m
@@ -0,0 +1,16 @@
+function specLocation = getEmbeddedSpecLocation(filename, options)
+% getEmbeddedSpecLocation - Get location of embedded specs in NWB file
+%
+%   Note: Returns an empty string if the spec location does not exist
+%
+%   See also io.spec.internal.readEmbeddedSpecLocation
+
+    arguments
+        filename (1,1) string {matnwb.common.mustBeNwbFile}
+        options.SpecLocAttributeName (1,1) string = '.specloc'
+    end
+
+    fid = H5F.open(filename);
+    fileCleanup = onCleanup(@(id) H5F.close(fid) );
+    specLocation = io.spec.internal.readEmbeddedSpecLocation(fid, options.SpecLocAttributeName);
+end
diff --git a/+io/+spec/readEmbeddedSpecifications.m b/+io/+spec/readEmbeddedSpecifications.m
@@ -0,0 +1,60 @@
+function specs = readEmbeddedSpecifications(filename, specLocation)
+% readEmbeddedSpecifications - Read embedded specs from an NWB file
+%
+%   specs = io.spec.readEmbeddedSpecifications(filename, specLocation) read 
+%       embedded specs from the specLocation in an NWB file
+%
+%   Inputs:
+%       filename (string) : Absolute path of an nwb file
+%       specLocation (string) : h5 path for the location of specs inside the NWB file
+%
+%   Outputs
+%       specs cell: A cell array of structs with one element for each embedded
+%           specification. Each struct has two fields:
+%
+%       - namespaceName (char)      : Name of the namespace for a specification
+%       - namespaceText (char)      : The namespace declaration for a specification
+%       - schemaMap (containers.Map): A set of schema specifications for the namespace
+
+    arguments
+        filename (1,1) string {matnwb.common.mustBeNwbFile}
+        specLocation (1,1) string
+    end
+
+    specInfo = h5info(filename, specLocation);
+    specs = deal( cell(size(specInfo.Groups)) );
+
+    fid = H5F.open(filename);
+    fileCleanup = onCleanup(@(id) H5F.close(fid) );
+
+    for iGroup = 1:length(specInfo.Groups)
+        location = specInfo.Groups(iGroup).Groups(1);
+
+        namespaceName = split(specInfo.Groups(iGroup).Name, '/');
+        namespaceName = namespaceName{end};
+
+        filenames = {location.Datasets.Name};
+        if ~any(strcmp('namespace', filenames))
+            warning('NWB:Read:GenerateSpec:CacheInvalid',...
+                'Couldn''t find a `namespace` in namespace `%s`.  Skipping cache generation.',...
+                namespaceName);
+            return;
+        end
+        sourceNames = {location.Datasets.Name};
+        fileLocation = strcat(location.Name, '/', sourceNames);
+        schemaMap = containers.Map;
+        for iFileLocation = 1:length(fileLocation)
+            did = H5D.open(fid, fileLocation{iFileLocation});
+            if strcmp('namespace', sourceNames{iFileLocation})
+                namespaceText = H5D.read(did);
+            else
+                schemaMap(sourceNames{iFileLocation}) = H5D.read(did);
+            end
+            H5D.close(did);
+        end
+
+        specs{iGroup}.namespaceName = namespaceName;
+        specs{iGroup}.namespaceText = namespaceText;
+        specs{iGroup}.schemaMap = schemaMap;
+    end
+end
diff --git a/+io/+spec/writeEmbeddedSpecifications.m b/+io/+spec/writeEmbeddedSpecifications.m
@@ -0,0 +1,45 @@
+function writeEmbeddedSpecifications(fid, jsonSpecs)
+    specLocation = io.spec.internal.readEmbeddedSpecLocation(fid);
+
+    if isempty(specLocation)
+        specLocation = '/specifications';
+        io.writeGroup(fid, specLocation);
+        specView = types.untyped.ObjectView(specLocation);
+        io.writeAttribute(fid, '/.specloc', specView);
+    end
+
+    for iJson = 1:length(jsonSpecs)
+        JsonDatum = jsonSpecs(iJson);
+        schemaNamespaceLocation = strjoin({specLocation, JsonDatum.name}, '/');
+        namespaceExists = io.writeGroup(fid, schemaNamespaceLocation);
+        if namespaceExists
+            namespaceGroupId = H5G.open(fid, schemaNamespaceLocation);
+            names = getVersionNames(namespaceGroupId);
+            H5G.close(namespaceGroupId);
+            for iNames = 1:length(names)
+                H5L.delete(fid, [schemaNamespaceLocation '/' names{iNames}],...
+                    'H5P_DEFAULT');
+            end
+        end
+        schemaLocation =...
+            strjoin({schemaNamespaceLocation, JsonDatum.version}, '/');
+        io.writeGroup(fid, schemaLocation);
+        Json = JsonDatum.json;
+        schemeNames = keys(Json);
+        for iScheme = 1:length(schemeNames)
+            name = schemeNames{iScheme};
+            path = [schemaLocation '/' name];
+            io.writeDataset(fid, path, Json(name));
+        end
+    end
+end
+
+function versionNames = getVersionNames(namespaceGroupId)
+    [~, ~, versionNames] = H5L.iterate(namespaceGroupId,...
+        'H5_INDEX_NAME', 'H5_ITER_NATIVE',...
+        0, @removeGroups, {});
+    function [status, versionNames] = removeGroups(~, name, versionNames)
+        versionNames{end+1} = name;
+        status = 0;
+    end
+end
diff --git a/+io/parseGroup.m b/+io/parseGroup.m
@@ -76,9 +76,7 @@
             parsed = NwbFile(kwargs{:});
         else
             file.cloneNwbFileClass(Type.name, Type.typename);
-            rehash();
             parsed = io.createParsedType(info.Name, Type.typename, kwargs{:});
-
         end
 
         return;