Repository navigation
Conversation
NWBIO fails to read most NWB files created by other software: fields that are links to other NWB objects or string datasets are copied into annotations, containers and image series in acquisition are treated as TimeSeries, units without obs_intervals or spike_times and intervals without tags raise errors, and lazy loading fails because the NWB proxies have no parent links. Writing renames every signal and event in place, does not filter nwb: annotations against the target class, ignores offset and channel_conversion, and does not write signal descriptions. The generic BaseTestIO tests never ran for NWBIO because its mode was "one-file". It is now "file", the NeuroConv and tutorial files already in ephy_testing_data are added to the tested entities, and new tests that build NWB files with pynwb run without network access.
NWBIO does not store the description of a SpikeTrain. It is now written to a _description column of the units table, with an empty string representing None, and read back from that column when present. This partly addresses NeuralEnsemble#1516.
NWBIO uses its AnalogSignalProxy for both regularly and irregularly sampled signals, but the class declares itself a proxy for AnalogSignal, so with lazy=True, adding the proxy for an irregularly sampled signal to a Segment raises a TypeError. The proxy now declares IrregularlySampledSignal when the TimeSeries has no sampling rate.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
NWBIOfails on most NWB files written by other software, including the file in #1413, whereread()raisesValueError: Invalid annotation … StrDataset. I sampled nine small files from the DANDI Archive, covering ecephys, icephys and ophys; eight of them fail onmaster. Lazy loading fails even for files that read eagerly. Writing renames the Neo objects in place and drops signal and SpikeTrain descriptions (part of #1516).The generic
BaseTestIOchecks also never run for NWBIO, because itsmodeis"one-file", which the test framework doesn't recognise.Solution
NWBIOnow reads these files. NWB objects it doesn't yet support (processing modules,ImageSeries, extension types, data with more than two dimensions) are skipped with a log message instead of raising an error. Values that aren't valid Neo annotations, such as links to other NWB objects, are skipped. String datasets are converted tostr.Behaviour changes that may affect existing code:
offsetandchannel_conversionare applied when reading, and aconversionthat isn't a power of ten is applied to the data. These three fields no longer appear asnwb:annotations.notes: JSON in anAnnotationSeriesdescription goes into the Event'snotesannotation, as it already does for signals, instead of being merged into the top-level annotations.comments: thecommentsof a foreign TimeSeries are available asnwb:commentsand are kept when writing.nwb:annotations on write: annotations the target NWB class doesn't accept are dropped with a warning.TimeSeries. One example is anElectricalSeries, because writing electrodes isn't supported yet.obs_intervals:t_startandt_stopcome from the range of all spike times in the table. A unit with several observation intervals keeps them innwb:obs_intervals.IndexError.Fixes #1413, partly addresses #1516, and covers the
obs_intervalsitem of #1013 and thekeywordspart of #1717. It supersedes the draft #1502.Testing
NWBIO.modeis now"file", so theBaseTestIOcompliance checks run. They run against the NeuroConv and tutorial NWB files already inephy_testing_data.NEO_TESTS_NO_NETWORK.Other comments
This PR makes foreign files readable but doesn't yet preserve their structure. Processing modules, extra table columns, electrode tables and icephys tables are still lost. I'm working on a follow-up PR for this.