From 628df2014c17729de6b2ce43fad4efc716cf50fa Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 13:40:46 +0200 Subject: [PATCH 1/9] feat: add schema violation reporting source context --- .../+validation/+internal/reportingSource.m | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 +matnwb/+common/+validation/+internal/reportingSource.m diff --git a/+matnwb/+common/+validation/+internal/reportingSource.m b/+matnwb/+common/+validation/+internal/reportingSource.m new file mode 100644 index 00000000..f8a59131 --- /dev/null +++ b/+matnwb/+common/+validation/+internal/reportingSource.m @@ -0,0 +1,40 @@ +function previousSource = reportingSource(newSource) +% reportingSource - Get or set the current read-validation reporting source. + + arguments + newSource = [] + end + + persistent activeSource + + if isempty(activeSource) + activeSource = []; + end + + previousSource = activeSource; + + if nargin > 0 + validateSource(newSource) + activeSource = newSource; + end +end + +function validateSource(source) + if isempty(source) + return + end + + assert(isstruct(source) && isscalar(source), ... + 'NWB:Validation:InvalidReportingSource', ... + 'Reporting source must be a scalar struct.') + assert(isfield(source, 'TypeName') && isfield(source, 'Path'), ... + 'NWB:Validation:InvalidReportingSource', ... + 'Reporting source must have TypeName and Path fields.') + assert(isTextScalar(source.TypeName) && isTextScalar(source.Path), ... + 'NWB:Validation:InvalidReportingSource', ... + 'Reporting source TypeName and Path fields must be text scalars.') +end + +function tf = isTextScalar(value) + tf = ischar(value) || (isstring(value) && isscalar(value)); +end From 2dc817044036c096d84b84def6f2438732623bfd Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 13:41:39 +0200 Subject: [PATCH 2/9] feat: include read source in schema warnings --- .../+validation/reportSchemaViolation.m | 7 +++++ .../+validation/ReportSchemaViolationTest.m | 29 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/+matnwb/+common/+validation/reportSchemaViolation.m b/+matnwb/+common/+validation/reportSchemaViolation.m index 9c4fcc29..227fd2e4 100644 --- a/+matnwb/+common/+validation/reportSchemaViolation.m +++ b/+matnwb/+common/+validation/reportSchemaViolation.m @@ -31,6 +31,13 @@ function reportSchemaViolation(errorId, message, causes, options) for iCause = 1:numel(causes) fullMessage = fullMessage + " " + string(causes(iCause).message); end + source = matnwb.common.validation.internal.reportingSource(); + if isReadContext && ~isempty(source) + sourceMessage = sprintf( ... + 'While reading object of type "%s" at file location "%s".', ... + source.TypeName, source.Path); + fullMessage = fullMessage + " " + sourceMessage; + end fullMessage = fullMessage + " " + lenientGuidance; warning(errorId, '%s', fullMessage) diff --git a/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m b/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m index fe40773a..6586bb92 100644 --- a/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m +++ b/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m @@ -4,6 +4,7 @@ methods (TestMethodTeardown) function resetValidationContext(~) matnwb.common.validation.internal.context("edit"); + matnwb.common.validation.internal.reportingSource([]); end end @@ -50,10 +51,34 @@ function testReadContextWarnsWithGuidanceAndCauseMessages(testCase) warningMessage, 'The nested validation failed.') testCase.verifySubstring( ... warningMessage, 'The non-conforming value is kept.') + testCase.verifyFalse(contains(warningMessage, 'While reading object')) + end + + function testReadContextWarnsWithReportingSource(testCase) + matnwb.common.validation.internal.context("read"); + source.TypeName = 'types.core.TimeSeries'; + source.Path = '/acquisition/bad_ts'; + matnwb.common.validation.internal.reportingSource(source); + + lastwarn('') + testCase.verifyWarning( ... + @() matnwb.common.validation.reportSchemaViolation( ... + 'NWB:Test:SchemaViolation', ... + "The value does not match the schema."), ... + 'NWB:Test:SchemaViolation') + + [warningMessage, warningId] = lastwarn; + testCase.verifyEqual(warningId, 'NWB:Test:SchemaViolation') + testCase.verifySubstring(warningMessage, ... + ['While reading object of type "types.core.TimeSeries" ' ... + 'at file location "/acquisition/bad_ts".']) end function testWarnInsteadOfErrorWarnsInEditContext(testCase) matnwb.common.validation.internal.context("edit"); + source.TypeName = 'types.core.TimeSeries'; + source.Path = '/acquisition/bad_ts'; + matnwb.common.validation.internal.reportingSource(source); testCase.verifyWarning( ... @() matnwb.common.validation.reportSchemaViolation( ... @@ -61,6 +86,10 @@ function testWarnInsteadOfErrorWarnsInEditContext(testCase) "The value does not match the schema.", ... WarnInsteadOfError=true), ... 'NWB:Test:SchemaViolation') + + [warningMessage, warningId] = lastwarn; + testCase.verifyEqual(warningId, 'NWB:Test:SchemaViolation') + testCase.verifyFalse(contains(warningMessage, 'While reading object')) end function testWriteContextRaisesErrorWithWarnInsteadOfError(testCase) From a2a424c49f965052af84a0ccbb7bd96316f17350 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 13:41:59 +0200 Subject: [PATCH 3/9] refactor: return scoped validation context cleanup --- .../+common/+validation/+internal/context.m | 12 ++++++- .../+validation/+internal/reportingSource.m | 32 +++++++++++++++---- NwbFile.m | 5 +-- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/+matnwb/+common/+validation/+internal/context.m b/+matnwb/+common/+validation/+internal/context.m index dda68994..92d6c64a 100644 --- a/+matnwb/+common/+validation/+internal/context.m +++ b/+matnwb/+common/+validation/+internal/context.m @@ -1,5 +1,10 @@ -function previousContext = context(newContext) +function [previousContext, cleanup] = context(newContext) % context - Get or set the process-local schema validation context. +% +% [~, cleanup] = context(newContext) additionally returns an onCleanup +% handle that restores the prior context when it goes out of scope. +% cleanup must be assigned to a named variable — if ignored, it fires +% immediately and the state change is immediately undone. arguments newContext matnwb.common.validation.internal.ValidationContext = ... @@ -20,4 +25,9 @@ 'Validation context must be scalar.') activeContext = newContext; end + + if nargout > 1 + cleanup = onCleanup(@() ... + matnwb.common.validation.internal.context(previousContext)); + end end diff --git a/+matnwb/+common/+validation/+internal/reportingSource.m b/+matnwb/+common/+validation/+internal/reportingSource.m index f8a59131..aa84a5f6 100644 --- a/+matnwb/+common/+validation/+internal/reportingSource.m +++ b/+matnwb/+common/+validation/+internal/reportingSource.m @@ -1,8 +1,19 @@ -function previousSource = reportingSource(newSource) +function [previousSource, cleanup] = reportingSource(newSource, options) % reportingSource - Get or set the current read-validation reporting source. +% +% The new source can be provided either as a scalar struct with TypeName +% and Path fields, or as name-value pairs (TypeName=..., Path=...). +% The two forms cannot be combined. +% +% [~, cleanup] = reportingSource(newSource) additionally returns an onCleanup +% handle that restores the prior source when it goes out of scope. +% cleanup must be assigned to a named variable — if ignored, it fires +% immediately and the state change is immediately undone. arguments - newSource = [] + newSource struct = struct.empty % Struct with fields TypeName and Path + options.TypeName (1,1) string + options.Path (1,1) string end persistent activeSource @@ -13,16 +24,25 @@ previousSource = activeSource; - if nargin > 0 + if ~isempty(newSource) || ~isempty(fieldnames(options)) + assert(isempty(newSource) || isempty(fieldnames(options)), ... + 'NWB:Validation:InvalidReportingSource', ... + 'Specify source as a struct or as name-value pairs, not both.') + if isempty(newSource) + newSource = options; + end validateSource(newSource) activeSource = newSource; end + + if nargout > 1 + cleanup = onCleanup(@() ... + matnwb.common.validation.internal.reportingSource(previousSource)); + end end function validateSource(source) - if isempty(source) - return - end + if isempty(source); return; end assert(isstruct(source) && isscalar(source), ... 'NWB:Validation:InvalidReportingSource', ... diff --git a/NwbFile.m b/NwbFile.m index f88de796..cc70738e 100644 --- a/NwbFile.m +++ b/NwbFile.m @@ -41,9 +41,7 @@ function export(obj, filename, mode, options) options.StorageBackend (1,1) string = "hdf5" end - previousSchemaValidationContext = matnwb.common.validation.internal.context("write"); - schemaValidationContextCleanupObj = onCleanup( ... - @() matnwb.common.validation.internal.context(previousSchemaValidationContext)); + [~, contextCleanup] = matnwb.common.validation.internal.context("write"); % add to file create date if isa(obj.file_create_date, 'types.untyped.DataStub') @@ -82,7 +80,6 @@ function export(obj, filename, mode, options) rethrow(ME); end - clear schemaValidationContextCleanupObj end function datasetConfig = applyDatasetSettingsProfile(obj, profile, options) From fd5610ed84a9b0ae93f0362f3f87b0d1bc2b25a7 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 13:42:15 +0200 Subject: [PATCH 4/9] refactor: scope validation state during parsed reads --- +io/createParsedType.m | 8 +++----- +tests/+unit/+io/testCreateParsedType.m | 5 +++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/+io/createParsedType.m b/+io/createParsedType.m index cd27bc11..14a9f6ca 100644 --- a/+io/createParsedType.m +++ b/+io/createParsedType.m @@ -24,9 +24,9 @@ [lastWarningMessage, lastWarningID] = lastwarn('', ''); % Clear last warning - previousValidationContext = matnwb.common.validation.internal.context("read"); - validationContextCleanupObj = onCleanup( ... - @() matnwb.common.validation.internal.context(previousValidationContext)); + [~, contextCleanup] = matnwb.common.validation.internal.context("read"); %#ok + [~, sourceCleanup] = matnwb.common.validation.internal.reportingSource(... + "TypeName", typeName, "Path", typePath); %#ok try typeInstance = feval(typeName, varargin{:}); % Create the type. @@ -52,8 +52,6 @@ throw(newException) end - clear validationContextCleanupObj - [warningMessage, warningID] = lastwarn(); % Handle any warnings if they occurred. diff --git a/+tests/+unit/+io/testCreateParsedType.m b/+tests/+unit/+io/testCreateParsedType.m index 08aec592..8cb63479 100644 --- a/+tests/+unit/+io/testCreateParsedType.m +++ b/+tests/+unit/+io/testCreateParsedType.m @@ -5,6 +5,9 @@ function setupMethod(testCase) % Use a fixture to create a temporary working directory testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); + previousReportingSource = matnwb.common.validation.internal.reportingSource([]); + testCase.addTeardown( ... + @() matnwb.common.validation.internal.reportingSource(previousReportingSource)); end end @@ -63,6 +66,8 @@ function testCreateDynamicTableWithDuplicateColnamesWarns(testCase) dynamicTable = testCase.verifyWarning( ... @() io.createParsedType(testPath, testType, kwargs{:}), ... 'NWB:DynamicTable:DuplicateColumnNames'); + testCase.verifyEmpty( ... + matnwb.common.validation.internal.reportingSource()) testCase.verifyClass(dynamicTable, testType) [warningMessage, warningIdentifier] = lastwarn(); From cb4c92b16dd03c3bda96933c3dbc8cd43e394d80 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 13:42:55 +0200 Subject: [PATCH 5/9] refactor: report unset properties at validation source --- +io/createParsedType.m | 43 --------------------------------------- +types/+util/checkUnset.m | 22 +++++++++++++++++--- 2 files changed, 19 insertions(+), 46 deletions(-) diff --git a/+io/createParsedType.m b/+io/createParsedType.m index 14a9f6ca..a8e601dc 100644 --- a/+io/createParsedType.m +++ b/+io/createParsedType.m @@ -5,10 +5,6 @@ % 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. @@ -19,11 +15,6 @@ % 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 - [~, contextCleanup] = matnwb.common.validation.internal.context("read"); %#ok [~, sourceCleanup] = matnwb.common.validation.internal.reportingSource(... "TypeName", typeName, "Path", typePath); %#ok @@ -31,16 +22,10 @@ try typeInstance = feval(typeName, varargin{:}); % Create the type. catch exception - lastwarn(lastWarningMessage, lastWarningID); % Reset last warning - - % Add information about which data type failed, and where in the - % file it is located. newException = MException('NWB:createParsedType:TypeCreationFailed', ... 'Failed to create object of type "%s" in file location "%s".', ... typeName, typePath); - % Add full error stack to the exception's cause for easier - % debugging extendedCause = MException(exception.identifier, ... getReport(exception, "extended")); newException = newException.addCause(extendedCause); @@ -52,32 +37,4 @@ throw(newException) end - [warningMessage, warningID] = lastwarn(); - - % Handle any warnings if they occurred. - 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/+types/+util/checkUnset.m b/+types/+util/checkUnset.m index bc9a5e04..13b17d18 100644 --- a/+types/+util/checkUnset.m +++ b/+types/+util/checkUnset.m @@ -15,8 +15,24 @@ function checkUnset(obj, argin) end dropped = setdiff(argin, union(allProperties, anonNames)); if ~isempty(dropped) - warning('NWB:CheckUnset:InvalidProperties', ... - 'Unexpected properties {%s} for instance of type "%s".', ... + message = sprintf('Unexpected properties {%s} for instance of type "%s".', ... misc.cellPrettyPrint(dropped), class(obj)); + + source = matnwb.common.validation.internal.reportingSource(); + if ~isempty(source) + message = sprintf('%s at file location "%s".', ... + message(1:end-1), source.Path); + end + + message = sprintf('%s\nNB: The properties in question were dropped.', message); + + if matnwb.common.validation.isReadContext() + message = sprintf(['%s\nConsider 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'], ... + message); + end + + warning('NWB:CheckUnset:InvalidProperties', '%s', message) end -end \ No newline at end of file +end From 8d7b24bca0e407f2896a4c9c226e33915e5b4c0f Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 13:51:12 +0200 Subject: [PATCH 6/9] refactor: rename reporting source to validation target --- +io/createParsedType.m | 2 +- .../+validation/+internal/reportingSource.m | 60 ------------------- .../+validation/+internal/validationTarget.m | 60 +++++++++++++++++++ .../+validation/reportSchemaViolation.m | 10 ++-- .../+validation/ReportSchemaViolationTest.m | 8 +-- +tests/+unit/+io/testCreateParsedType.m | 6 +- +types/+util/checkUnset.m | 6 +- 7 files changed, 76 insertions(+), 76 deletions(-) delete mode 100644 +matnwb/+common/+validation/+internal/reportingSource.m create mode 100644 +matnwb/+common/+validation/+internal/validationTarget.m diff --git a/+io/createParsedType.m b/+io/createParsedType.m index a8e601dc..e0c46296 100644 --- a/+io/createParsedType.m +++ b/+io/createParsedType.m @@ -16,7 +16,7 @@ % typeInstance - The generated neurodata type instance. [~, contextCleanup] = matnwb.common.validation.internal.context("read"); %#ok - [~, sourceCleanup] = matnwb.common.validation.internal.reportingSource(... + [~, targetCleanup] = matnwb.common.validation.internal.validationTarget(... "TypeName", typeName, "Path", typePath); %#ok try diff --git a/+matnwb/+common/+validation/+internal/reportingSource.m b/+matnwb/+common/+validation/+internal/reportingSource.m deleted file mode 100644 index aa84a5f6..00000000 --- a/+matnwb/+common/+validation/+internal/reportingSource.m +++ /dev/null @@ -1,60 +0,0 @@ -function [previousSource, cleanup] = reportingSource(newSource, options) -% reportingSource - Get or set the current read-validation reporting source. -% -% The new source can be provided either as a scalar struct with TypeName -% and Path fields, or as name-value pairs (TypeName=..., Path=...). -% The two forms cannot be combined. -% -% [~, cleanup] = reportingSource(newSource) additionally returns an onCleanup -% handle that restores the prior source when it goes out of scope. -% cleanup must be assigned to a named variable — if ignored, it fires -% immediately and the state change is immediately undone. - - arguments - newSource struct = struct.empty % Struct with fields TypeName and Path - options.TypeName (1,1) string - options.Path (1,1) string - end - - persistent activeSource - - if isempty(activeSource) - activeSource = []; - end - - previousSource = activeSource; - - if ~isempty(newSource) || ~isempty(fieldnames(options)) - assert(isempty(newSource) || isempty(fieldnames(options)), ... - 'NWB:Validation:InvalidReportingSource', ... - 'Specify source as a struct or as name-value pairs, not both.') - if isempty(newSource) - newSource = options; - end - validateSource(newSource) - activeSource = newSource; - end - - if nargout > 1 - cleanup = onCleanup(@() ... - matnwb.common.validation.internal.reportingSource(previousSource)); - end -end - -function validateSource(source) - if isempty(source); return; end - - assert(isstruct(source) && isscalar(source), ... - 'NWB:Validation:InvalidReportingSource', ... - 'Reporting source must be a scalar struct.') - assert(isfield(source, 'TypeName') && isfield(source, 'Path'), ... - 'NWB:Validation:InvalidReportingSource', ... - 'Reporting source must have TypeName and Path fields.') - assert(isTextScalar(source.TypeName) && isTextScalar(source.Path), ... - 'NWB:Validation:InvalidReportingSource', ... - 'Reporting source TypeName and Path fields must be text scalars.') -end - -function tf = isTextScalar(value) - tf = ischar(value) || (isstring(value) && isscalar(value)); -end diff --git a/+matnwb/+common/+validation/+internal/validationTarget.m b/+matnwb/+common/+validation/+internal/validationTarget.m new file mode 100644 index 00000000..25a1ea72 --- /dev/null +++ b/+matnwb/+common/+validation/+internal/validationTarget.m @@ -0,0 +1,60 @@ +function [previousTarget, cleanup] = validationTarget(newTarget, options) +% validationTarget - Get or set the current schema-validation target. +% +% The new target can be provided either as a scalar struct with TypeName +% and Path fields, or as name-value pairs (TypeName=..., Path=...). +% The two forms cannot be combined. +% +% [~, cleanup] = validationTarget(newTarget) additionally returns an onCleanup +% handle that restores the prior target when it goes out of scope. +% cleanup must be assigned to a named variable — if ignored, it fires +% immediately and the state change is immediately undone. + + arguments + newTarget struct = struct.empty % Struct with fields TypeName and Path + options.TypeName (1,1) string + options.Path (1,1) string + end + + persistent activeTarget + + if isempty(activeTarget) + activeTarget = []; + end + + previousTarget = activeTarget; + + if ~isempty(newTarget) || ~isempty(fieldnames(options)) + assert(isempty(newTarget) || isempty(fieldnames(options)), ... + 'NWB:Validation:InvalidValidationTarget', ... + 'Specify target as a struct or as name-value pairs, not both.') + if isempty(newTarget) + newTarget = options; + end + validateTarget(newTarget) + activeTarget = newTarget; + end + + if nargout > 1 + cleanup = onCleanup(@() ... + matnwb.common.validation.internal.validationTarget(previousTarget)); + end +end + +function validateTarget(target) + if isempty(target); return; end + + assert(isstruct(target) && isscalar(target), ... + 'NWB:Validation:InvalidValidationTarget', ... + 'Validation target must be a scalar struct.') + assert(isfield(target, 'TypeName') && isfield(target, 'Path'), ... + 'NWB:Validation:InvalidValidationTarget', ... + 'Validation target must have TypeName and Path fields.') + assert(isTextScalar(target.TypeName) && isTextScalar(target.Path), ... + 'NWB:Validation:InvalidValidationTarget', ... + 'Validation target TypeName and Path fields must be text scalars.') +end + +function tf = isTextScalar(value) + tf = ischar(value) || (isstring(value) && isscalar(value)); +end diff --git a/+matnwb/+common/+validation/reportSchemaViolation.m b/+matnwb/+common/+validation/reportSchemaViolation.m index 227fd2e4..39fc52db 100644 --- a/+matnwb/+common/+validation/reportSchemaViolation.m +++ b/+matnwb/+common/+validation/reportSchemaViolation.m @@ -31,12 +31,12 @@ function reportSchemaViolation(errorId, message, causes, options) for iCause = 1:numel(causes) fullMessage = fullMessage + " " + string(causes(iCause).message); end - source = matnwb.common.validation.internal.reportingSource(); - if isReadContext && ~isempty(source) - sourceMessage = sprintf( ... + target = matnwb.common.validation.internal.validationTarget(); + if isReadContext && ~isempty(target) + targetMessage = sprintf( ... 'While reading object of type "%s" at file location "%s".', ... - source.TypeName, source.Path); - fullMessage = fullMessage + " " + sourceMessage; + target.TypeName, target.Path); + fullMessage = fullMessage + " " + targetMessage; end fullMessage = fullMessage + " " + lenientGuidance; diff --git a/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m b/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m index 6586bb92..90f17ae7 100644 --- a/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m +++ b/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m @@ -4,7 +4,7 @@ methods (TestMethodTeardown) function resetValidationContext(~) matnwb.common.validation.internal.context("edit"); - matnwb.common.validation.internal.reportingSource([]); + matnwb.common.validation.internal.validationTarget([]); end end @@ -54,11 +54,11 @@ function testReadContextWarnsWithGuidanceAndCauseMessages(testCase) testCase.verifyFalse(contains(warningMessage, 'While reading object')) end - function testReadContextWarnsWithReportingSource(testCase) + function testReadContextWarnsWithValidationTarget(testCase) matnwb.common.validation.internal.context("read"); source.TypeName = 'types.core.TimeSeries'; source.Path = '/acquisition/bad_ts'; - matnwb.common.validation.internal.reportingSource(source); + matnwb.common.validation.internal.validationTarget(source); lastwarn('') testCase.verifyWarning( ... @@ -78,7 +78,7 @@ function testWarnInsteadOfErrorWarnsInEditContext(testCase) matnwb.common.validation.internal.context("edit"); source.TypeName = 'types.core.TimeSeries'; source.Path = '/acquisition/bad_ts'; - matnwb.common.validation.internal.reportingSource(source); + matnwb.common.validation.internal.validationTarget(source); testCase.verifyWarning( ... @() matnwb.common.validation.reportSchemaViolation( ... diff --git a/+tests/+unit/+io/testCreateParsedType.m b/+tests/+unit/+io/testCreateParsedType.m index 8cb63479..88618e14 100644 --- a/+tests/+unit/+io/testCreateParsedType.m +++ b/+tests/+unit/+io/testCreateParsedType.m @@ -5,9 +5,9 @@ function setupMethod(testCase) % Use a fixture to create a temporary working directory testCase.applyFixture(matlab.unittest.fixtures.WorkingFolderFixture); - previousReportingSource = matnwb.common.validation.internal.reportingSource([]); + previousValidationTarget = matnwb.common.validation.internal.validationTarget([]); testCase.addTeardown( ... - @() matnwb.common.validation.internal.reportingSource(previousReportingSource)); + @() matnwb.common.validation.internal.validationTarget(previousValidationTarget)); end end @@ -67,7 +67,7 @@ function testCreateDynamicTableWithDuplicateColnamesWarns(testCase) @() io.createParsedType(testPath, testType, kwargs{:}), ... 'NWB:DynamicTable:DuplicateColumnNames'); testCase.verifyEmpty( ... - matnwb.common.validation.internal.reportingSource()) + matnwb.common.validation.internal.validationTarget()) testCase.verifyClass(dynamicTable, testType) [warningMessage, warningIdentifier] = lastwarn(); diff --git a/+types/+util/checkUnset.m b/+types/+util/checkUnset.m index 13b17d18..f3ec014a 100644 --- a/+types/+util/checkUnset.m +++ b/+types/+util/checkUnset.m @@ -18,10 +18,10 @@ function checkUnset(obj, argin) message = sprintf('Unexpected properties {%s} for instance of type "%s".', ... misc.cellPrettyPrint(dropped), class(obj)); - source = matnwb.common.validation.internal.reportingSource(); - if ~isempty(source) + target = matnwb.common.validation.internal.validationTarget(); + if ~isempty(target) message = sprintf('%s at file location "%s".', ... - message(1:end-1), source.Path); + message(1:end-1), target.Path); end message = sprintf('%s\nNB: The properties in question were dropped.', message); From f0308ee0a2664ea68fa574f11434258223c3cd9e Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 15:06:14 +0200 Subject: [PATCH 7/9] Improve warning message composition when including a read target context --- .../+validation/reportSchemaViolation.m | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/+matnwb/+common/+validation/reportSchemaViolation.m b/+matnwb/+common/+validation/reportSchemaViolation.m index 39fc52db..83aa9a5a 100644 --- a/+matnwb/+common/+validation/reportSchemaViolation.m +++ b/+matnwb/+common/+validation/reportSchemaViolation.m @@ -24,20 +24,24 @@ function reportSchemaViolation(errorId, message, causes, options) isWriteContext = validationContext == ValidationContext.WRITE; if ~isWriteContext && (isReadContext || options.WarnInsteadOfError) - lenientGuidance = ['The non-conforming value is kept. If you maintain ' ... - 'this data, consider correcting it before export.']; + lenientGuidance = sprintf("\nNote: The non-conforming value is kept. If you maintain " + ... + "this data, consider correcting it before export."); + + fullMessage = ""; - fullMessage = message; - for iCause = 1:numel(causes) - fullMessage = fullMessage + " " + string(causes(iCause).message); - end target = matnwb.common.validation.internal.validationTarget(); if isReadContext && ~isempty(target) targetMessage = sprintf( ... - 'While reading object of type "%s" at file location "%s".', ... + 'Validation failed while reading object of type "%s" at file location "%s":', ... target.TypeName, target.Path); - fullMessage = fullMessage + " " + targetMessage; + fullMessage = string(targetMessage) + newline; end + + fullMessage = fullMessage + message; + for iCause = 1:numel(causes) + fullMessage = fullMessage + " " + string(causes(iCause).message); + end + fullMessage = fullMessage + " " + lenientGuidance; warning(errorId, '%s', fullMessage) From 615dadb2866e708529bab979a7a518d6b37d1f79 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 15:28:05 +0200 Subject: [PATCH 8/9] Fix updated warning text in validation test --- +tests/+unit/+common/+validation/ReportSchemaViolationTest.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m b/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m index 90f17ae7..5f0e3fa3 100644 --- a/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m +++ b/+tests/+unit/+common/+validation/ReportSchemaViolationTest.m @@ -70,8 +70,8 @@ function testReadContextWarnsWithValidationTarget(testCase) [warningMessage, warningId] = lastwarn; testCase.verifyEqual(warningId, 'NWB:Test:SchemaViolation') testCase.verifySubstring(warningMessage, ... - ['While reading object of type "types.core.TimeSeries" ' ... - 'at file location "/acquisition/bad_ts".']) + ['Validation failed while reading object of type ' ... + '"types.core.TimeSeries" at file location "/acquisition/bad_ts":']) end function testWarnInsteadOfErrorWarnsInEditContext(testCase) From 3abec75ed83486fb2ffc84cf7ce10a03ae3a3bf4 Mon Sep 17 00:00:00 2001 From: ehennestad Date: Wed, 1 Jul 2026 15:36:18 +0200 Subject: [PATCH 9/9] Fix validationTarget context reset, and add regression test --- .../+validation/+internal/validationTarget.m | 11 +++-- .../+validation/ValidationTargetTest.m | 49 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 +tests/+unit/+common/+validation/ValidationTargetTest.m diff --git a/+matnwb/+common/+validation/+internal/validationTarget.m b/+matnwb/+common/+validation/+internal/validationTarget.m index 25a1ea72..0e693f22 100644 --- a/+matnwb/+common/+validation/+internal/validationTarget.m +++ b/+matnwb/+common/+validation/+internal/validationTarget.m @@ -19,16 +19,21 @@ persistent activeTarget if isempty(activeTarget) - activeTarget = []; + activeTarget = struct.empty; end previousTarget = activeTarget; - if ~isempty(newTarget) || ~isempty(fieldnames(options)) + % Any explicit argument is a set/reset request; calling with no arguments + % is a pure getter. nargin counts only positional arguments, so name-value + % input is detected via the options struct. An explicit empty target resets + % the state, which is how the onCleanup handler restores the prior (possibly + % empty) target when a scope exits. + if nargin >= 1 || ~isempty(fieldnames(options)) assert(isempty(newTarget) || isempty(fieldnames(options)), ... 'NWB:Validation:InvalidValidationTarget', ... 'Specify target as a struct or as name-value pairs, not both.') - if isempty(newTarget) + if isempty(newTarget) && ~isempty(fieldnames(options)) newTarget = options; end validateTarget(newTarget) diff --git a/+tests/+unit/+common/+validation/ValidationTargetTest.m b/+tests/+unit/+common/+validation/ValidationTargetTest.m new file mode 100644 index 00000000..5976437f --- /dev/null +++ b/+tests/+unit/+common/+validation/ValidationTargetTest.m @@ -0,0 +1,49 @@ +classdef ValidationTargetTest < matlab.unittest.TestCase +% ValidationTargetTest - Unit tests for the schema-validation target state. + + methods (TestMethodTeardown) + function resetValidationTarget(~) + matnwb.common.validation.internal.validationTarget([]); + end + end + + methods (Test) + function testExplicitEmptyResetsState(testCase) + % Passing an explicit empty target must clear the stored state, + % not be treated as a no-op getter. + target.TypeName = 'types.core.TimeSeries'; + target.Path = '/acquisition/bad_ts'; + matnwb.common.validation.internal.validationTarget(target); + testCase.verifyNotEmpty( ... + matnwb.common.validation.internal.validationTarget()) + + matnwb.common.validation.internal.validationTarget([]); + testCase.verifyEmpty( ... + matnwb.common.validation.internal.validationTarget()) + end + + function testCleanupRestoresEmptyTarget(testCase) + % When a scope sets a target from the initial empty state, its + % cleanup handle must restore the state back to empty on exit. + target.TypeName = 'types.core.TimeSeries'; + target.Path = '/acquisition/bad_ts'; + + % The cleanup handle is scoped to the helper, so it fires when the + % helper returns, mimicking how a read scope exits. + testCase.setScopedTarget(target) + + testCase.verifyEmpty( ... + matnwb.common.validation.internal.validationTarget()) + end + end + + methods (Access = private) + function setScopedTarget(testCase, target) + [~, cleanup] = matnwb.common.validation.internal ... + .validationTarget(target); %#ok + testCase.verifyEqual( ... + matnwb.common.validation.internal.validationTarget().TypeName, ... + target.TypeName) + end + end +end