Skip to content

Move the Base.save to Sorting.save() and Recording.save() - #4671

Merged
alejoe91 merged 30 commits into
SpikeInterface:mainfrom
samuelgarcia:refactor_base_save_dump
Sep 17, 2026
Merged

alejoe91 merged 30 commits into
SpikeInterface:mainfrom
samuelgarcia:refactor_base_save_dump

Conversation

@samuelgarcia

@samuelgarcia samuelgarcia commented Jul 9, 2026

Copy link
Copy Markdown
Member

This PR refactors the BaseExtractor.save() function to unify and simplify the implementation:

  • Remove the _save() function and deprecate save_to_memory, save_to_folder, save_to_zarr: the save() function is now the only entry point for saving
  • The BaseRecording and BaseSorting route the save to the main backend formats, which have a write_* statich method to handle all the write logic (e.g., BinaryFolderRecording.write_recording())
  • Remove the folder_metadata concept, used previously to save the probe object and not needed since Use ProbeGroup object instead of contact_vector property and set_probe/probegroup from select_channels_with_probe/probegroup #4465 (now the probegroup is dumped to dictionary as extra metadata)
  • Centralize saving and loading of properties and annotations to folder
  • Deprecate BaseSnippets object, since it's not an intended use case we want to suppor in the future

In addition to these changes, this PR also integrates #4689, which improves the handling of times in several ways:

  • it adds time info to the dict only when it has been modified in memory
  • it changes the write_binary function to also write timestamps in chunks when available
  • it modifies FrameSliceRecoridng and BasePreprocessor to lazily handleand return timestamps from parent, instead od making a full copy

move saving logic to extractor classes
@samuelgarcia samuelgarcia added the Edinburgh hackathon 2026 PRs from Edinburgh hackathon 2026 label Jul 9, 2026
@alejoe91 alejoe91 changed the title mode the Base.save to Sorting.save() and Recording.save() Move the Base.save to Sorting.save() and Recording.save() Jul 15, 2026
@alejoe91 alejoe91 added this to the 0.106.0 milestone Jul 17, 2026
@samuelgarcia
samuelgarcia marked this pull request as ready for review September 8, 2026 11:00
@alejoe91

alejoe91 commented Sep 8, 2026

Copy link
Copy Markdown
Member

@samuelgarcia fixed some bugs in preprocessing, but there are more. Do you want me to fix it?

@alejoe91

alejoe91 commented Sep 9, 2026

Copy link
Copy Markdown
Member

@samuelgarcia ported changes from #4689 (timestamps handling + removing metadata_folder) and cleaned up code and docs.

One main question: in #4689 , I added an extra _in_reduce mechanism to the to_dict/_extra_metadata_to_dict stack. This was only set to True by the __reduce__, and propagated times kwargs only in this case.

Here I propose a modified version: if the user "messes" with timestamps, this is propagated to dict no matter what. Note that:

  • this is needed when saving in parallel (timestamps need to be saved!)
  • the final si_folder.json will NOT have this information, since it sets include_extra_metadate=False (added also include_annotations/include_properties to False)

Let me know what you think!

@alejoe91 alejoe91 added core Changes to core module refactor Refactor of code, with no change to functionality labels Sep 9, 2026
@alejoe91 alejoe91 modified the milestones: 0.106.0, 0.105.0 Sep 9, 2026
Comment thread src/spikeinterface/core/binaryrecordingextractor.py
Comment thread src/spikeinterface/core/basesorting.py Outdated
Comment thread src/spikeinterface/core/time_series.py Outdated
Comment thread src/spikeinterface/preprocessing/basepreprocessor.py
dump_dict["annotations"] = {
k: self._annotations.get(k) for k in self._main_annotations if self._annotations.get(k) is not None
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to save the main_ids somewhere near here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 443f80a

@alejoe91

Copy link
Copy Markdown
Member

@samuelgarcia @chrishalcrow all done! Ready to merge on my end.

Comment thread src/spikeinterface/core/base.py Outdated
@alejoe91
alejoe91 merged commit b16c672 into SpikeInterface:main Sep 17, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes to core module Edinburgh hackathon 2026 PRs from Edinburgh hackathon 2026 refactor Refactor of code, with no change to functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants