From c51099fa1c9b96fd0648d26da2ba345b357ffe6e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Fri, 13 Sep 2024 21:12:23 +0200 Subject: [PATCH] 592 improve warning message at types util check unset (#593) * Adress issue #592 * Update docstring and comments for io.createParsedType * Add unittest for io.createParsedType * Update createParsedType.m Fix: avoid showing warning twice --- +io/createParsedType.m | 58 +++++++++++++++++++++++++ +io/parseDataset.m | 2 +- +io/parseGroup.m | 5 ++- +tests/+unit/+io/testCreateParsedType.m | 35 +++++++++++++++ +types/+util/checkUnset.m | 10 ++--- 5 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 +io/createParsedType.m create mode 100644 +tests/+unit/+io/testCreateParsedType.m diff --git a/+io/createParsedType.m b/+io/createParsedType.m new file mode 100644 index 00000000..2454a750 --- /dev/null +++ b/+io/createParsedType.m @@ -0,0 +1,58 @@ +function typeInstance = createParsedType(typePath, typeName, varargin) +% createParsedType - Create a neurodata type from a specified type name +% +% This function generates a neurodata type instance from a given type name +% and a corresponding cell array of name-value pairs. It is typically used +% when parsing datasets or groups. +% +% Warnings with the ID "NWB:CheckUnset:InvalidProperties" are captured, and +% the warning message is enhanced with specific details about the dataset or +% group in the NWB file where the issue occurred. +% +% Inputs: +% typePath - (char) Path to the dataset or group in the NWB file where the +% neurodata type is parsed from. +% typeName - (char) Name of the neurodata type to be created. +% varargin - (cell) Cell array of name-value pairs representing the +% properties of the neurodata type. +% +% Outputs: +% typeInstance - The generated neurodata type instance. + + + warnState = warning('off', 'NWB:CheckUnset:InvalidProperties'); + cleanupObj = onCleanup(@(s) warning(warnState)); % Make sure warning state is reset later + + [lastWarningMessage, lastWarningID] = lastwarn('', ''); % Clear last warning + + typeInstance = feval(typeName, varargin{:}); % Create the type. + + [warningMessage, warningID] = lastwarn(); + + % Handle any warnings if they occured. + if ~isempty(warningMessage) + if strcmp( warningID, 'NWB:CheckUnset:InvalidProperties' ) + + clear cleanupObj % Reset last warning state + + if endsWith(warningMessage, '.') + warningMessage = warningMessage(1:end-1); + end + + updatedMessage = sprintf('%s at file location "%s"\n', warningMessage, typePath); + + disclaimer = 'NB: The properties in question were dropped while reading the file.'; + + suggestion = [... + 'Consider checking the schema version of the file with '... + '`util.getSchemaVersion(filename)` and comparing with the ' ... + 'YAML namespace version present in nwb-schema/core/nwb.namespace.yaml' ]; + + warning(warningID, '%s\n%s\n\n%s', updatedMessage, disclaimer, suggestion) + else + % Pass, warning has already been displayed + end + else + lastwarn(lastWarningMessage, lastWarningID); % Reset last warning + end +end diff --git a/+io/parseDataset.m b/+io/parseDataset.m index 7bd8fd90..80195a10 100644 --- a/+io/parseDataset.m +++ b/+io/parseDataset.m @@ -78,7 +78,7 @@ else props('data') = data; kwargs = io.map2kwargs(props); - parsed = eval([Type.typename '(kwargs{:})']); + parsed = io.createParsedType(fullpath, Type.typename, kwargs{:}); end H5D.close(did); H5F.close(fid); diff --git a/+io/parseGroup.m b/+io/parseGroup.m index 76c2ccbc..46a87c6c 100644 --- a/+io/parseGroup.m +++ b/+io/parseGroup.m @@ -77,12 +77,13 @@ else file.cloneNwbFileClass(Type.name, Type.typename); rehash(); - parsed = eval([Type.typename '(kwargs{:})']); + parsed = io.createParsedType(info.Name, Type.typename, kwargs{:}); + end return; end - parsed = eval([Type.typename '(kwargs{:})']); + parsed = io.createParsedType(info.Name, Type.typename, kwargs{:}); end end diff --git a/+tests/+unit/+io/testCreateParsedType.m b/+tests/+unit/+io/testCreateParsedType.m new file mode 100644 index 00000000..7980162f --- /dev/null +++ b/+tests/+unit/+io/testCreateParsedType.m @@ -0,0 +1,35 @@ +function tests = testCreateParsedType() + tests = functiontests(localfunctions); +end + +function setupOnce(testCase) + rootPath = fullfile(fileparts(mfilename('fullpath')), '..', '..', '..'); + testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath)); + + testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + generateCore('savedir', '.') +end + +function testCreateTypeWithValidInputs(testCase) + testPath = 'some/dataset/path'; + testType = 'types.hdmf_common.VectorIndex'; + kwargs = {'description', 'this is a test'}; + + type = io.createParsedType(testPath, testType, kwargs{:}); + testCase.verifyClass(type, testType) + + testCase.verifyWarningFree(... + @(varargin)io.createParsedType(testPath, testType, kwargs{:})) +end + +function testCreateTypeWithInvalidInputs(testCase) + testPath = 'some/dataset/path'; + testType = 'types.hdmf_common.VectorIndex'; + kwargs = {'description', 'this is a test', 'comment', 'this is another test'}; + type = io.createParsedType(testPath, testType, kwargs{:}); + testCase.verifyClass(type, testType) + + testCase.verifyWarning(... + @(varargin)io.createParsedType(testPath, testType, kwargs{:}), ... + 'NWB:CheckUnset:InvalidProperties') +end \ No newline at end of file diff --git a/+types/+util/checkUnset.m b/+types/+util/checkUnset.m index 9ba8eb57..bc9a5e04 100644 --- a/+types/+util/checkUnset.m +++ b/+types/+util/checkUnset.m @@ -15,12 +15,8 @@ function checkUnset(obj, argin) end dropped = setdiff(argin, union(allProperties, anonNames)); if ~isempty(dropped) - warning('NWB:CheckUnset:InvalidProperties' ... - , ['Unexpected properties {%s}. '... - '\n\nYour schema version may be incompatible with the file. '... - 'Consider checking the schema version of the file with '... - '`util.getSchemaVersion(filename)` '... - 'and comparing with the YAML namespace version present in '... - 'nwb-schema/core/nwb.namespace.yaml'], misc.cellPrettyPrint(dropped)); + warning('NWB:CheckUnset:InvalidProperties', ... + 'Unexpected properties {%s} for instance of type "%s".', ... + misc.cellPrettyPrint(dropped), class(obj)); end end \ No newline at end of file