NWBHDF5IO ER and ER_Manager

[mavaylon1]

Motivation

The two changes is to add the ability to set an instance of ExternalResources for the NWBFile and NWBHDF5IO.
This is the first integration of ExternalResources into pynwb. The file is able to be linked to an instance of ExternalResources, while remaining separate during read/write.

How to test the behavior?

Show how to reproduce the new behavior (can be a bug fix or a new feature)

There is a test for resources via test_resources.py. The NWBFile operates as it previously did with added functionality to support ExternalResources.

Checklist

• Did you update CHANGELOG.md with your changes?
• Have you checked our Contributing document?
• Have you ensured the PR clearly describes the problem and the solution?
• Is your contribution compliant with our coding style? This can be checked running flake8 from the source directory.
• Have you checked to ensure that there aren't other open Pull Requests for the same change?
• Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

[codecov]

Codecov Report

Merging #1684 (fc56216) into dev (aae7d78) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1684      +/-   ##
==========================================
+ Coverage   91.93%   91.96%   +0.03%     
==========================================
  Files          26       27       +1     
  Lines        2592     2602      +10     
  Branches      679      680       +1     
==========================================
+ Hits         2383     2393      +10     
  Misses        134      134              
  Partials       75       75              

Flag	Coverage Δ
integration	71.32% <42.85%> (-0.20%)	⬇️
unit	83.62% <100.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/pynwb/__init__.py	78.48% <100.00%> (+0.13%)	⬆️
src/pynwb/file.py	88.88% <100.00%> (+0.02%)	⬆️
src/pynwb/resources.py	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[oruebel]

Other than deciding whether adding external_resources should be moved to HDMF, this PR looks good to me.

[mavaylon1]

@oruebel I wanted to go a bit further with the ER logic in (hdmf-dev/hdmf#895) where the external_resources variable would be set to a read instance from the path, but this is not possible due HDMFIO and HDF5IO being imported in the hdmf/common init.py for namespace validation and will throw a circular import. As a result, the solution I came up with is that the variable is set to the path in HDMFIO and HDF5IO, but instantiated here. We need to merge the hdmf ticket, do a release, and change the minimum hdmf version here. Thoughts?

[oruebel]

namespace validation and will throw a circular import.

If I understand it correctly you only need the ExternalResources in the read method. You could just do a local import of ExternalResources inside the read method itself to avoid the circular import.

[mavaylon1]

namespace validation and will throw a circular import.

If I understand it correctly you only need the ExternalResources in the read method. You could just do a local import of ExternalResources inside the read method itself to avoid the circular import.

namespace validation and will throw a circular import.

If I understand it correctly you only need the ExternalResources in the read method. You could just do a local import of ExternalResources inside the read method itself to avoid the circular import.

namespace validation and will throw a circular import.

If I understand it correctly you only need the ExternalResources in the read method. You could just do a local import of ExternalResources inside the read method itself to avoid the circular import.

Local import makes sense. Pending approval of the hdmf PR.

[mavaylon1]

@oruebel can you do one last look through? Not much changed since after your approval, but I removed the logic for linking resources since that moved to hdmf, I updated the reqs to the release of hdmf 3.7, and I added an ignore to codecov for resources.py.

[rly]

I added an ignore to codecov for resources.py.

Why ignore coverage testing of this file?

[mavaylon1]

There's nothing to test and coverage was flagging it.

Edit: created a test similar to test_core.py