Add read_nwb_method for local paths in both hdf5 and zarr

[h-mayorquin]

Motivation

See #1974

How to test the behavior?

I added tests.

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 ruff check . && codespell 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

All modified and coverable lines are covered by tests ✅

Project coverage is 91.73%. Comparing base (3d04646) to head (43b9bba).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1994      +/-   ##
==========================================
+ Coverage   91.69%   91.73%   +0.04%     
==========================================
  Files          27       27              
  Lines        2708     2722      +14     
  Branches      707      710       +3     
==========================================
+ Hits         2483     2497      +14     
  Misses        149      149              
  Partials       76       76              

Flag	Coverage Δ
integration	72.96% <100.00%> (+0.13%)	⬆️
unit	82.29% <14.28%> (-0.36%)	⬇️

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

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

[stephprince]

Sorry for the delay @h-mayorquin! Added my review comments

[h-mayorquin]

Is the sphinx links fail related to this PR? Given my experience with neuroconv is seldom is and I looked at the CI log but no luck.

[stephprince]

Looks good to me, thanks! Added a few more small suggestions related to the docstrings.

[h-mayorquin]

Implemented the changes but now there is more CI trouble it seems.

[stephprince]

The coverage report is saying that read_nwb had no test coverage. It seems the new test file is not being run with the other integration tests.

Since we still run tests using test.py and haven't migrated completely to pytest (see #1803), I think adding run_test_suite("tests/integration/io", "integration io tests", verbose=verbose) to test.py here should ensure the new code is being tested in the CI.

[h-mayorquin]

Thanks @stephprince . I added this.

run_test_suite("tests/integration/io", "integration io tests", verbose=verbose)

Will this handle both the cases with and without hdmf-zarr? as it, include them in the coverage.

[stephprince]

Will this handle both the cases with and without hdmf-zarr? as it, include them in the coverage.

I think so. The coverage is run with and without optional requirements (which includes hdmf-zarr)

[h-mayorquin]

Yeah, I am confused about the difference between codecov/patch and codecov/project. The latter works fine but the former indicates that the test is not run at all. It seems:

[stephprince]

I don't think the updates to add the integration io tests to test.py got pushed, I added them and the __init__.py that I think is necessary from testing locally.

From my understanding codecov/project is the coverage for all of pynwb when applying the PR changes, and patch is the coverage for the lines that were specifically changed. I'm not sure exactly how the coverage reports are merged and reported within the PR, maybe @rly knows in more detail?

[stephprince]

Looks like we are good now though and testing is running with and without hdmf-zarr. Thanks for the work on this!

[h-mayorquin]

I don't think the updates to add the integration io tests to test.py got pushed, I added them and the __init__.py that I think is necessary from testing locally.

My bad, I realize now -painfully- that I pushed those changes to other branch : /
Thanks for fixing my mistake here.

From my understanding codecov/project is the coverage for all of pynwb when applying the PR changes, and patch is the coverage for the lines that were specifically changed. I'm not sure exactly how the coverage reports are merged and reported within the PR, maybe @rly knows in more detail?

Thanks for the info and all the help on this PR!