Separate NWB reading from extractor construction#4630
Open
h-mayorquin wants to merge 7 commits into
Open
Conversation
Replace the parallel _pynwb/_backend methods in the NWB recording path with a single _NwbGeneralReader (reading_method discriminator). The reader owns open/locate/close and the electrode-column reads; the extractor keeps only the SpikeInterface mapping (uV scaling, brain_area rename, core-vs-extra column policy). Sorting and time-series extractors are unchanged.
Migrate NwbSortingExtractor onto the single _NWBReader: add the units side (load_units, unit_ids, spike_times, spike_times_index, units_column_names, available_units_tables, rate_and_t_start_from_electrical_series) and thread storage_options through _open_handle. The sorting extractor drops its parallel _fetch_sorting_segment_info_pynwb/_backend pair, self.backend, and the _BaseNWBExtractor mixin; the ragged-property logic in _fetch_properties is preserved, just sourcing units_table from the reader. Time-series extractor unchanged.
Migrate NwbTimeSeriesExtractor onto the single _NWBReader (load_timeseries / available_timeseries) and retire the _BaseNWBExtractor mixin: no extractor inherits it anymore, so file-handle cleanup lives entirely in _NWBReader.close()/__del__ shared by all three extractors via composition. Also drop a stray no-op '1' line. All three NWB extractors now speak one idiom.
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
2 participants
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.
I am planning to fix the following open issues:
Before doing so I want to do a refactor. The nwb reading logic (three formats, two streaming modes) is currently tangled with the extractor construction logic for all three extractors (recording, sorting, time-series), so a fix in reading requires finding and changing multiple duplicated sites. This PR separates those two concerns so I can do a centralized fix.
This will fix #2910