Move the Base.save to Sorting.save() and Recording.save() - #4671
Move the Base.save to Sorting.save() and Recording.save()#4671samuelgarcia wants to merge 17 commits into
Base.save to Sorting.save() and Recording.save()#4671Conversation
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.
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.