592 improve warning message at types util check unset

ehennestad commented 2 months ago

Fix #592

Motivation

More informative warning message when parsing datasets

How to test the behavior?

download the following file: https://api.dandiarchive.org/api/assets/b4ca3863-2892-4706-af75-e5650fed46eb/download/

nwbRead('path/to/file.nwb')

Checklist

[x] Have you ensured the PR description clearly describes the problem and solutions?
[x] Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
[x] If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.60%. Comparing base (4b280cb) to head (916e168). Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
+io/parseGroup.m	50.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #593 +/- ## ========================================== + Coverage 90.57% 90.60% +0.02% ========================================== Files 105 106 +1 Lines 4657 4671 +14 ========================================== + Hits 4218 4232 +14 Misses 439 439 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bendichter commented 2 months ago

@ehennestad it would be helpful if you could include a bit more explanation in these PRs.

On the master branch I get that this reads correctly with a warning about a description that was unexpected, but not where it is in the file:

nwbRead('/Users/bendichter/dev/sub-mouse-INETJ_ses-20200407-sample-10_slice-20200407-slice-1_cell-20200407-sample-10_icephys (1).nwb')
Warning: Unexpected properties {description}.

Your 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 
> In types.util.checkUnset (line 18)
In types.hdmf_common/VectorIndex (line 23)
In io.parseDataset (line 81)
In io.parseGroup (line 22)
In io.parseGroup (line 38)
In io.parseGroup (line 38)
In io.parseGroup (line 38)
In nwbRead (line 79) 

ans = 

  NwbFile with properties:

                                nwb_version: '2.1.0'
                          ...

after the change the warning is improved to:

Warning: Unexpected properties {description} for instance of type
"types.hdmf_common.VectorIndex" at file location
"/general/intracellular_ephys/sweep_table/series_index"

NB: The properties in question were dropped while reading 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 
> In io.createParsedType (line 51)
In io.parseDataset (line 81)
In io.parseGroup (line 22)
In io.parseGroup (line 38)
In io.parseGroup (line 38)
In io.parseGroup (line 38)
In nwbRead (line 79)

which is much better because it tells me exactly where the error is coming from.

The error is improved and that is the point of this PR so that's great. But while we are here let's also look at the schema violation.

VectorIndex is defined here:

There is indeed a description. Now let's see what the rules are. VectorIndex is defined here. No description. Now let's see it's parent, VectorData, defined here, which DOES define a description!

https://github.com/hdmf-dev/hdmf-common-schema/blob/5b4cbb31dbafcff51ca70bf218f464b186568151/common/table.yaml#L4-L40

So why does MatNWB not expect it? Maybe this is based on a different version of the schema?

ehennestad commented 2 months ago

The names and version of the embedded namespaces for that file is:	Name	Version
'NWB core'	'2.1.0'
'HDMF Common'	'1.1.3'
'ndx-dandi-icephys'	'0.3.0'

There are two things that are off here: 1) HDMF Common v1.1.3 was part of NWB v2.2.2 2) VectorIndex in HDMF-common v1.1.3 has Index as parent, none of them defines description as an attribute

NeurodataWithoutBorders / matnwb