Fix bugs related to reading logical and enum data types (#758)

* Add private props to store dims and type of DataStub

These values should ideally be set on construction (they are immutable?) to avoid repeated calls to h5 library

* Update parseDataset.m

Create DataStub with size and type information available from h5info structure

* Update parseDataset.m

Add handling for compound data type

* Update parseDataset.m

Add handling for boolean / logical data type

* Update parseDataset.m

Include character set value for char type

* Improve datatypeInfoToMatlabType

Removed 'keyboard' debugging statements and added a default empty output for unresolved types in datatypeInfoToMatlabType. This makes the function safer and avoids unexpected interruptions during execution.

* Minor: Fix argument in CheckDtypeTest for map input

Corrects the name argument from 'table' to 'map' in the CheckDtypeTest unit test when verifying containers.Map input.

* Support and validate HDF5 compound types in DataStub

Enhances DataStub and related utilities to handle HDF5 compound types by storing and validating detailed type descriptors as structs. Adds recursive validation for compound type structure in checkDtype, updates getMatType to extract compound type descriptors, and ensures correct handling and error reporting for compound datasets throughout the loading and validation process.

* Handle compound HDF5 types in datatypeInfoToMatlabType

Added extractCompoundTypeDescriptor to support parsing HDF5 compound types into MATLAB struct type descriptors. This enables correct handling of nested compound, string, enum, and reference types when converting HDF5 datatypes to MATLAB representations.

* Set default value for dataType_ property

Initialized the private property dataType_ to string.empty in the DataStub class to ensure it has a default value.

* Update test to expect struct class for compound type

Changed the testTableType unit test to verify that io.getMatType returns a 'struct' class instead of 'table' for H5T_COMPOUND types. This reflects a change in expected behavior as data type of a compound should be represented with a structure instead 'table' char

* Update checkDtype.m

For numeric types, we should not require strict type match

* Update Dataset.m

* Improve HDF5 reference type handling in datatype parsing

Enhances the datatypeInfoToMatlabType function to explicitly distinguish between 'H5R_OBJECT' and 'H5R_DATASET_REGION' reference types, mapping them to ObjectView and RegionView, respectively. Refactors extractCompoundTypeDescriptor to recursively use datatypeInfoToMatlabType for consistent type detection

* Update nwbtest.m

* Refactor datatypeInfoToMatlabType to new internal module

Moved and refactored the datatypeInfoToMatlabType function from parseDataset.m to +io/+internal/+h5/+datatype/datatypeInfoToMatlabType.m for better modularity and maintainability.
Updated parseDataset.m to use the new function location.
Removed the old function implementation from parseDataset.m.

* Update datatypeInfoToMatlabType.m

* Replace error with assert for compound type validation

Changed the validation for compound type descriptors from an error to an assert statement in DataStub.m.

* Update checkDtype.m

Removed unused function
Improved errors

* Added utility functions for easier use of test schemas in tests

* Update dataStubTest.m

* Add function to cast values to logical

- Added function to cast values read with low-level or high-level h5 functions to logical vectors
- Use this new function in io.parseDataset and io.parseCompound

* Update parseAttributes.m

- Fixed verb from save to read as this function is part of the read pipeline
- Fixed bug where utput value was not assigned, leading to error

* Update isBool.m

- Combine logical expressions using && to break early if condition is not met
- Fixes a bug where logical comparison error if Type.Member has a different length than 2
- Added comment about likely bug

* Handle enum types in getMatType

Updated getMatType to distinguish between boolean and other enum types. Now, if the type is an enum and not a boolean, it returns 'cell'

* Update datatypeInfoToMatlabType.m

* Add unit tests for enum parsing in io.parseDataset

Introduces EnumTest to verify correct parsing of HDF5 enum datasets, including boolean and unknown enum types, in the io.parseDataset and io.parseAttributes functions. Tests cover scalar and array datasets, attribute parsing, and ensure appropriate warnings and data type conversions are handled.

* Update EnumTest.m

Comment out failed test for coverage reporting

* Update parseDataset.m

* Refactor enum to logical conversion in HDF5 parsing

Replaces direct string comparison with a call to io.internal.h5.cast.toLogical for converting enum values to logicals in both parseAttributes and DataStub.

* Add function to convert enum values to cell array of strings

Introduces toEnumCellStr, a utility for converting HDF5 enum integer values or cell arrays of strings to a cell array of enum member names, supporting both h5info Datatype structs and H5ML.id type identifiers. Includes helper functions for building enum value-to-name maps from both struct and type ID inputs.

* Update enum postprocessing

-Fixed bug in toEnumCellStr
- Use toEnumCellStr in relevant locations
Update test

* Renamed "io.internal.h5.cast.toEnumCellStr" to "io.internal.h5.postprocess.toEnumCellStr".

* Renamed "io.internal.h5.cast.toLogical" to "io.internal.h5.postprocess.toLogical".

* Update toEnumCellStr.m

---------

Co-authored-by: Ben Dichter <[email protected]>

Author: ehennestad

+io/+internal/+h5/+datatype/datatypeInfoToMatlabType.m

@@ -51,8 +51,9 @@
             else
                 warning('NWB:Dataset:UnknownEnum', ...
                     ['Encountered unknown enum under field `%s` with %d members. ' ...
-                    'Will be saved as cell array of characters.'], ...
+                    'Will be read as cell array of characters.'], ...
                     datasetName, length(datatype.Type.Member));
+                matlabDataType = 'cell';
             end
         end
     end

+io/+internal/+h5/+postprocess/toEnumCellStr.m

@@ -0,0 +1,79 @@
+function cellValue = toEnumCellStr(value, dataType)
+% toEnumCellStr - Convert enum integer values to cell array of strings.
+%
+% Syntax:
+%   cellValue = io.internal.h5.postprocess.toEnumCellStr(value, dataType) 
+%   Converts the given enum integer values to their corresponding string 
+%   representations based on the datatype definition.
+%
+% Input Arguments:
+%   value - The input enum value(s) to be converted. Can be a scalar or 
+%   array of int8 values, or a cell array of strings (from h5read).
+%
+%   dataType - A Datatype structure containing the enum definition a returned
+%   by h5info or a H5ML.id (type identifier).
+%
+% Output Arguments:
+%   cellValue - Cell array of strings containing the enum member names
+%   corresponding to the input values.
+%
+% Note: Low level h5 functions (H5D.read) return enum values as int8,
+% whereas high level functions (i.e h5read) return enum values as cell 
+% arrays of character vectors. This function accepts both types as input.
+
+    % If already a cell array of strings, return as-is
+    if iscellstr(value) %#ok<ISCLSTR> - We don't expect string arrays here
+        cellValue = value;
+        return;
+    end
+
+    % Build a lookup map from enum values to names
+    if isstruct(dataType)
+        enumMap = enumMapFromTypeStruct(dataType);
+    elseif isa(dataType, 'H5ML.id')
+        enumMap = enumMapFromTypeId(dataType);
+    end
+
+    % Validate values, ensuring complete representation by enum members
+    enumValues = enumMap.keys();
+    enumValues = cast([enumValues{:}], 'like', value);
+
+    assert(all(ismember(value, enumValues)), ...
+        'NWB:CastH5ToEnumCellStr:UnknownValue', ...
+        ['Enum data values do not match the enum member values in the ', ...
+        'enum type definition'])
+       
+    % Convert values to cell array of strings
+    valueSize = size(value);
+    cellValue = cell(valueSize);
+
+    for i = 1:numel(enumValues)
+        IND = value == enumValues(i);
+        cellValue(IND) = deal({enumMap(enumValues(i))});
+    end
+end
+
+function enumMap = enumMapFromTypeStruct(dataType)
+% Build a lookup map from enum values to names from h5info Datatype struct
+    enumMap = containers.Map('KeyType', 'int32', 'ValueType', 'char');
+    for i = 1:length(dataType.Member)
+        memberValue = dataType.Member(i).Value;
+        memberName = dataType.Member(i).Name;
+        enumMap(int32(memberValue)) = memberName;
+    end
+end
+
+function enumMap = enumMapFromTypeId(typeId)
+% Build a lookup map from enum values to names using HDF5 type ID
+    enumMap = containers.Map('KeyType', 'int32', 'ValueType', 'char');
+    
+    % Get number of enum members
+    numMembers = H5T.get_nmembers(typeId);
+    
+    % Iterate through each member (0-indexed)
+    for iMember = 0:(numMembers-1)
+        memberName = H5T.get_member_name(typeId, iMember);
+        memberValue = H5T.get_member_value(typeId, iMember);
+        enumMap(int32(memberValue)) = memberName;
+    end
+end

+io/+internal/+h5/+postprocess/toLogical.m

@@ -0,0 +1,29 @@
+function logicalValue = toLogical(value)
+% toLogical - Convert input value to logical.
+%
+% Syntax:
+%   logicalValue = io.internal.h5.postprocess.toLogical(value) Converts the given 
+%   h5 value to a logical value based on its type.
+%
+% Input Arguments:
+%   value - The input value to be converted. It can be of type int8 or 
+%   a cell containing a string representation of boolean values ("TRUE" or 
+%   "FALSE").
+%
+% Output Arguments:
+%   logicalValue - The corresponding logical value (true or false) after 
+%   conversion.
+%
+% Note: Low level h5 functions (H5D.read) returns enum values as int8,
+% whereas high level functions (i.e h5read) returns enum values as a cell 
+% arrays of character vectors. This function accepts both types as input
+% and returns a logical vector
+
+    if isa(value, 'int8')
+        logicalValue = logical(value);
+    elseif isa(value, 'cell') && ismember(string(value{1}), ["TRUE", "FALSE"])
+        logicalValue = strcmp('TRUE', value);
+    else
+        error('NWB:CastH5ToLogical:UnknownLogicalFormat', 'Could not resolve data of logical type')
+    end
+end

+io/getMatType.m

@@ -26,8 +26,12 @@
     type = 'types.untyped.ObjectView';
 elseif H5T.equal(tid, 'H5T_STD_REF_DSETREG')
     type = 'types.untyped.RegionView';
-elseif io.isBool(tid)
-    type = 'logical';
+elseif H5T.get_class(tid) == H5ML.get_constant_value('H5T_ENUM')
+    if io.isBool(tid)
+        type = 'logical';
+    else
+        type = 'cell';
+    end
 elseif H5ML.get_constant_value('H5T_COMPOUND') == H5T.get_class(tid)
     type = extractCompoundTypeDescriptor(tid);
 else

+io/isBool.m

@@ -12,12 +12,13 @@
 end
 
 function tf = isBoolFromTypeStruct(Type)
-tf = all([...
-    strcmp('H5T_STD_I8LE', Type.Type),...
-    2 == length(Type.Member),...
-    all(strcmp({'FALSE', 'TRUE'}, sort({Type.Member.Name}))),...
-    all([0,1] == sort([Type.Member.Value]))...
-    ]);
+tf = strcmp('H5T_STD_I8LE', Type.Type) ...
+     && length(Type.Member) == 2 ...
+     && all(strcmp({'FALSE', 'TRUE'}, sort({Type.Member.Name}))) ...
+     && isequal([0,1], sort([Type.Member.Value]));
+% Note: 
+% Sorting values seems like a bug. This will allow FALSE = 1 and TRUE = 0
+% (Although very unlikely to happen in practice)
 end
 
 function tf = isBoolFromTypeId(tid)

+io/parseAttributes.m

@@ -37,7 +37,7 @@
 for i=1:length(attributes)
     attr = attributes(i);
 
-    switch attr.Datatype.Class
+    switch attr.Datatype.Class % Normalize/postprocess some HDF5 classes
         case 'H5T_STRING'
             % H5 String type attributes are loaded differently in releases 
             % prior to MATLAB R2020a. For details, see:
@@ -64,14 +64,13 @@
             H5F.close(fid);
         case 'H5T_ENUM'
             if io.isBool(attr.Datatype.Type)
-                % attr.Value should be cell array of strings here since
-                % MATLAB can't have arbitrary enum values.
-                attributeValue = strcmp('TRUE', attr.Value);
+                attributeValue = io.internal.h5.postprocess.toLogical(attr.Value);
             else
                 warning('NWB:Attribute:UnknownEnum', ...
                     ['Encountered unknown enum under field `%s` with %d members. ' ...
-                    'Will be saved as cell array of characters.'], ...
+                    'Will be read as cell array of characters.'], ...
                     attr.Name, length(attr.Datatype.Type.Member));
+                attributeValue = io.internal.h5.postprocess.toEnumCellStr(attr.Value, attr.Datatype.Type);
             end
         otherwise
             attributeValue = attr.Value;

+io/parseCompound.m

@@ -22,10 +22,14 @@
                 isCharacterType(iField) = ~H5T.is_variable_str(fieldTypeId);
             case H5ML.get_constant_value('H5T_ENUM')
                 isLogicalType(iField) = io.isBool(fieldTypeId);
+                % Note: There is currently no postprocessing applied for
+                % other ENUMs when parsing compound data types. 
+                % Should be fine as NWB only uses the ENUM class for booleans.
             otherwise
                 %do nothing
         end
     end
+    H5T.close(typeId)
 
     fieldName = fieldnames(data);
 
@@ -39,6 +43,11 @@
         data.(name) = io.parseReference(datasetId, rawTypeId, rawReference);
     end
 
+    % Close type ids
+    for i = 1:numel(subTypeId)
+        H5T.close(subTypeId{i})
+    end
+
     % transpose character arrays because they are column-ordered
     % when read
     characterFieldName = fieldName(isCharacterType);
@@ -51,12 +60,6 @@
     logicalFieldName = fieldName(isLogicalType);
     for iFieldName = 1:length(logicalFieldName)
         name = logicalFieldName{iFieldName};
-        if isa(data.(name), 'int8')
-            data.(name) = logical(data.(name));
-        elseif isa(data.(name), 'cell') && ismember(string(data.(name){1}), ["TRUE", "FALSE"])
-            data.(name) = strcmp('TRUE', data.(name));
-        else
-            error('NWB:ParseCompound:UnknownLogicalFormat', 'Could not resolve data of logical type')
-        end
+        data.(name) = io.internal.h5.postprocess.toLogical(data.(name));
     end
 end

+io/parseDataset.m

@@ -26,7 +26,7 @@
         tid = H5D.get_type(did);
         data = io.parseReference(did, tid, H5D.read(did));
         H5T.close(tid);
-    elseif ~strcmp(dataspace.Type, 'simple')
+    elseif ~strcmp(dataspace.Type, 'simple') % i.e scalar
         data = H5D.read(did);
 
         switch datatype.Class
@@ -43,12 +43,13 @@
                 end
             case 'H5T_ENUM'
                 if io.isBool(datatype.Type)
-                    data = strcmp('TRUE', data);
+                    data = io.internal.h5.postprocess.toLogical(data);
                 else
                     warning('NWB:Dataset:UnknownEnum', ...
                         ['Encountered unknown enum under field `%s` with %d members. ' ...
-                        'Will be saved as cell array of characters.'], ...
+                        'Will be read as cell array of characters.'], ...
                         name, length(datatype.Type.Member));
+                    data = io.internal.h5.postprocess.toEnumCellStr(data, datatype.Type);
                 end
         end
     else