Update warning about ignoring cached namespaces

[stephprince]

Motivation

Fix #1018.

Combine the warnings about ignoring cached namespaces into a single warning, and only warn for core namespaces if the cached version is newer than the loaded version.

How to test the behavior?

with an example file from the pynwb test suite:

from pynwb import NWBHDF5IO

io = NWBHDF5IO("tests/back_compat/2.1.0_nwbfile_with_extension.nwb", "r")
io.read()

UserWarning: Ignoring the following cached namespace(s) because another version is already loaded:
hdmf-experimental - cached version: 0.2.0, loaded version: 0.5.0
Ignore this warning if these versions are compatible.

Checklist

• Did you update CHANGELOG.md with your changes?
• Does the PR clearly describe the problem and the solution?
• Have you reviewed our Contributing Guide?
• Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

[codecov]

Codecov Report

Attention: Patch coverage is 97.80220% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.68%. Comparing base (6f138ec) to head (0e84263).
Report is 1 commits behind head on dev.

Files with missing lines	Patch %	Lines
src/hdmf/spec/namespace.py	97.40%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1258      +/-   ##
==========================================
+ Coverage   91.66%   91.68%   +0.01%     
==========================================
  Files          42       42              
  Lines        9552     9597      +45     
  Branches     1921     1933      +12     
==========================================
+ Hits         8756     8799      +43     
- Misses        518      519       +1     
- Partials      278      279       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rly]

I would really like to not show the hdmf-experimental warning but I know we do not ensure backwards compatibility between schema versions there. What if we also hide the warning if none of the data types in the namespace are used in the file? Most files do not involve data types used in hdmf-experimental.

On rewrite/append, the loaded namespaces are written to the file, so we should not have a problem with writing new data that is incompatible with old namespaces.

[stephprince]

I would really like to not show the hdmf-experimental warning but I know we do not ensure backwards compatibility between schema versions there. What if we also hide the warning if none of the data types in the namespace are used in the file? Most files do not involve data types used in hdmf-experimental.

I think that's a good idea! I agree it would be nice to not display that warning unless necessary.

Are there any places you can point me to in the code that get all the data types used in the file? I am thinking this would be an optional argument to NamespaceCatalog.load_namespaces since the NamespaceCatalog seems isolated from the file object?

[rly]

After talking with @oruebel , we realized that it would be hard or hacky to get all the data types used in the file. The hacky way is to iterate through all groups and datasets in the file and look for the neurodata_type or data_type attributes, but this involves iterating through the whole file just to check for a warning. This iteration is expensive, especially for streaming data.

Another approach we discussed is to have the IO object build all the builders before loading the namespace from the file. Then we can iterate through the in-memory builders. But this requires refactoring the logic in NWBHDF5IO and HDF5IO, which currently takes in a BuildManager that has the loaded namespace. This would be complicated.

The primary use case for the hdmf-experimental warning is to warn users of incompatible/breaking changes introduced in, say 0.4.0 when the data was written in an older version, say 0.3.0. This really only impacts ExternalResources. (EnumData is also in hdmf-experimental but has not changed). To my knowledge, no one is using older versions of ExternalResources (and perhaps also the latest version), it is relatively stable now, and no one has yet raised an issue about incompatibilities. Therefore the impact of just ignoring this warning is minimal.

So, I would say, let's include hdmf-experimental into the "core" set of namespaces to ignore from hdmf.

[stephprince]

So, I would say, let's include hdmf-experimental into the "core" set of namespaces to ignore from hdmf.

Thanks for explaining in detail. That sounds good to me - I'll add the hdmf-experimental namespace to the NamespaceCatalog.__core_namespace list.

Note that the pynwb __init__.py file will also need to be updated to add the "core" core namespace and fully eliminate the pynwb/hdmf namespace related warnings when the cached namespace version is not newer. (In the example output I demonstrated above with "tests/back_compat/2.1.0_nwbfile_with_extension.nwb", I think I was accidentally using a cached typemap in which I had added the "core" namespace in pynwb for testing purposes).

Edit: see NeurodataWithoutBorders/pynwb#2064 as the open issue to add "core" as a core namespace

[stephprince]

@rly this is ready for you to look at again

[stephprince]

@rly I think I addressed all of your comments

I believe the test failures are due to an ImportError issue with the schemasheets package. The warning in the logs is a little confusing because schemasheets is actually installed, but does not seem to be compatible with the latest versions of linkml_runtime>=1.9.0. I'll open up an issue on that repo (edit: see linkml/schemasheets#147).

[rly]

Thanks! I made a couple minor comments. Otherwise looks good.

[stephprince]

@rly I think I addressed all your remaining comments! This should be ready to merge

[rly]

Looks good. Thank you! Squash and merge anytime.

[rly]

I'll just do it since Matt and I are managing this package and the release.

[h-mayorquin]

This is great! Thanks for moving this forward!