-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix/read bad icephys table #919
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev #919 +/- ##
==========================================
- Coverage 88.33% 88.12% -0.21%
==========================================
Files 45 45
Lines 9283 9309 +26
Branches 2651 2661 +10
==========================================
+ Hits 8200 8204 +4
- Misses 765 782 +17
- Partials 318 323 +5
☔ View full report in Codecov by Sentry. |
try: | ||
from pynwb import NWBHDF5IO | ||
except ImportError: | ||
self.skipTest("PyNWB not installed for testing") | ||
test_filename = os.path.join(os.path.dirname(os.path.abspath(__file__)), | ||
"bad_nwb_icephys_aligned_dynamic_table_file.nwb") | ||
|
||
with NWBHDF5IO(test_filename, 'r', load_namespaces=True) as nwbio: | ||
with warnings.catch_warnings(record=True) as w: | ||
_ = nwbio.read() | ||
self.assertEqual(len(w), 7) |
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.
@rly the easiest way to test this is to just try an read the broken NWB file which has all the different errors. However, PyNWB is not installed for our CI, so this test gets skipped. Is there a way that we can read the file without just HDF5IO
?
Motivation
Fix #918
Relax the error checking in the
__init__
methods ofDynamicTable
andAlignedDynamicTable
to raise warnings when reading from file and raise errors when creating new data. This is to try and allow users to access bad data files while ensuring that we don't construct bad files ourselves.How to test the behavior?
See #918 for details
Checklist
ruff
from the source directory.