Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PyNWB 3.0 support #557

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Add PyNWB 3.0 support #557

wants to merge 4 commits into from

Conversation

stephprince
Copy link
Contributor

Motivation

Address deprecations and validation changes in the upcoming PyNWB 3.0 release.

Note: I left some remaining TODO items that can be updated after the official release when 3.0 becomes the new minimum for NWBInspector.

To test:

cd nwbinspector
pip install -e . 
pip install pynwb==3.0.0rc1
pytest

@codecov-commenter
Copy link

Codecov Report

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

Project coverage is 86.73%. Comparing base (33ca373) to head (3fee92f).

Files with missing lines Patch % Lines
src/nwbinspector/_nwb_inspection.py 75.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #557      +/-   ##
==========================================
+ Coverage   82.73%   86.73%   +3.99%     
==========================================
  Files          47       47              
  Lines        1512     1515       +3     
==========================================
+ Hits         1251     1314      +63     
+ Misses        261      201      -60     
Files with missing lines Coverage Δ
src/nwbinspector/checks/_images.py 91.30% <ø> (ø)
src/nwbinspector/_nwb_inspection.py 86.95% <75.00%> (+3.99%) ⬆️

... and 4 files with indirect coverage changes

@stephprince stephprince requested a review from rly January 29, 2025 22:40
@rly
Copy link
Contributor

rly commented Jan 30, 2025

Do you have any idea why the streaming tests are taking more than 1h 45m?

@stephprince
Copy link
Contributor Author

I'm not sure, I just noticed that. I can cancel the run for now and try to debug locally?

@rly
Copy link
Contributor

rly commented Jan 30, 2025

Sounds good.

Also, some time ago we had unofficially decided that NWB Zarr files should have the suffix ".nwb.zarr". I think we should keep that. NeurodataWithoutBorders/pynwb#1976 (comment) . NWB HDF5 files should still have suffix ".nwb" though. It can have the "hdf5" part before then to be explicit in these tests if we want.

@stephprince
Copy link
Contributor Author

Also, some time ago we had unofficially decided that NWB Zarr files should have the suffix ".nwb.zarr". I think we should keep that.

Ah right, thanks for catching. I was modifying the tests to address the new NWBHDF5IO warning to use the .nwb extension and forgot the recommendation was .nwb.zarr for Zarr files. I will fix those names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants