-
Notifications
You must be signed in to change notification settings - Fork 84
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 "bounds" to SpatialSeries fields #1907
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## nwb_schema_2.8.0 #1907 +/- ##
=================================================
Coverage 91.85% 91.85%
=================================================
Files 27 27
Lines 2689 2689
Branches 701 701
=================================================
Hits 2470 2470
Misses 145 145
Partials 74 74
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
Motivation
I noticed that pynwb tests were passing with NWB schema 2.7.0 even though "bounds" was not added to SpatialSeries until after NWB schema 2.7.0, so really the IO tests should fail.
Until then, on the dev branch, "bounds" in
SpatialSeries.__init__
will not be written to disk because it is missing from__nwbfields__
.Using the convenience test classes can hide this error. I think we should be more explicit when writing roundtrip tests to ensure that all expected fields are read correctly from the written file.
This PR merges into a new
nwb_schema_2.8.0
branch.Checklist
flake8
from the source directory.