Move the Base.save to Sorting.save() and Recording.save() - #4671
samuelgarcia wants to merge 18 commits into
Conversation
move saving logic to extractor classes
for more information, see https://pre-commit.ci
Base.save to Sorting.save() and Recording.save()
for more information, see https://pre-commit.ci
…keinterface into refactor_base_save_dump
for more information, see https://pre-commit.ci
… NumpyFolderSorting
…keinterface into refactor_base_save_dump
for more information, see https://pre-commit.ci
|
@samuelgarcia fixed some bugs in preprocessing, but there are more. Do you want me to fix it? |
|
@samuelgarcia ported changes from #4689 (timestamps handling + removing One main question: in #4689 , I added an extra Here I propose a modified version: if the user "messes" with timestamps, this is propagated to dict no matter what. Note that:
Let me know what you think! |
| gain_to_uV: float | np.ndarray | None = None, | ||
| offset_to_uV: float | np.ndarray | None = None, | ||
| is_filtered: bool | None = None, | ||
| file_timestamps_paths: str | Path | list[str | Path] | None = None, |
There was a problem hiding this comment.
I think we should limit the complexity of BinaryRecordingExtractr to load only one or sevreal binary files.
And keep the complexity only in BinaryFolderRecording other we need to maintained a 2 classes design.
| return None | ||
|
|
||
| def save(self, format="numpy_folder", **save_kwargs): | ||
| def save(self, format="binary", **save_kwargs): |
There was a problem hiding this comment.
Another suggestion is binary_folder, but revert for now
| # Flag to indicate whether time info has been modified in-memory (e.g. by set_times or shift_times). | ||
| _time_info_modified = False |
There was a problem hiding this comment.
Let's make a proper doubel inheritance and the init will set this to False
| # relying on the time_vector/t_start copied into __init__ above) lets any lazy/offset-aware | ||
| # override further up the chain (e.g. FrameSliceRecordingSegment after a frame_slice) keep | ||
| # working without materializing a full time_vector every time this segment is reconstructed. | ||
| def get_times(self, start_frame=None, end_frame=None): |
There was a problem hiding this comment.
we may need this for ChannelSlice too
Move saving logic to extractor classes.
For historical reason, lazyness and original bad design, the BaseExtractor.save() was a terrible mess for:
It was using some kind of "hook" to sub classes and the logic was almost impossible to follow.
I tried to move the logic save()/load() to classe (BinaryFolderRecording, ZarrRecordingExtractor, SharedMemoryRecording, ZarrSortingExtractor, NpzFolderSorting, NumpyFolderSorting).
The code looks quite strange at soe places but at least it is easier follow.
@alejoe91 : the time vector handling is not done correctly.
But your PR #4689 should be rebase on this one I think. Lets do it togoether.