From 9cdb7bcac18d08b3242f352a429b4a293314269e Mon Sep 17 00:00:00 2001 From: AxelNoun <150222552+AxelNoun@users.noreply.github.com> Date: Thu, 5 Nov 2026 22:08:59 +0100 Subject: [PATCH 1/3] Keep None in t_starts when saving a recording get_segment_t_starts returns a mixed list such as [None, 100.0] when only some segments have a start time, and three constructors called float() on every entry. Keep None in these constructors. Also set shms and main_shm_owner first in SharedMemoryRecording, so that __del__ is safe after a failed constructor, and return a float instead of a 0-d array from ZarrRecordingExtractor. Claude helped me with this change. --- .../core/binaryrecordingextractor.py | 9 +++--- src/spikeinterface/core/numpyextractors.py | 23 ++++++++------ .../core/tests/test_baserecording.py | 31 +++++++++++++++++++ .../core/tests/test_numpy_extractors.py | 25 +++++++++++++++ src/spikeinterface/core/zarrextractors.py | 2 +- 5 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/spikeinterface/core/binaryrecordingextractor.py b/src/spikeinterface/core/binaryrecordingextractor.py index 37f4ba3467..c67db4b416 100644 --- a/src/spikeinterface/core/binaryrecordingextractor.py +++ b/src/spikeinterface/core/binaryrecordingextractor.py @@ -26,8 +26,9 @@ class BinaryRecordingExtractor(BaseRecording): The dtype of the binary file time_axis : int, default: 0 The axis of the time dimension - t_starts : None or list of float, default: None - Times in seconds of the first sample for each segment + t_starts : None or list of float or None, default: None + Times in seconds of the first sample for each segment. An entry is None for a segment + without a start time. channel_ids : list, default: None A list of channel ids file_offset : int, default: 0 @@ -57,7 +58,7 @@ def __init__( sampling_frequency: float, dtype: str | np.dtype, num_channels: int | None = None, - t_starts: list[float] | None = None, + t_starts: list[float | None] | None = None, channel_ids: list[str | int] | None = None, time_axis: int = 0, file_offset: int = 0, @@ -83,7 +84,7 @@ def __init__( if t_starts is not None: assert len(t_starts) == len(file_path_list), "t_starts must be a list of the same size as file_paths" - t_starts = [float(t_start) for t_start in t_starts] + t_starts = [None if t_start is None else float(t_start) for t_start in t_starts] if file_timestamps_paths is not None: if isinstance(file_timestamps_paths, list): diff --git a/src/spikeinterface/core/numpyextractors.py b/src/spikeinterface/core/numpyextractors.py index 1ab942ec5c..9c421df351 100644 --- a/src/spikeinterface/core/numpyextractors.py +++ b/src/spikeinterface/core/numpyextractors.py @@ -25,8 +25,9 @@ class NumpyRecording(BaseRecording): The traces to instantiate a mono or multisegment Recording sampling_frequency : float The sampling frequency in Hz - t_starts : None or list of float - Times in seconds of the first sample for each segment + t_starts : None or list of float or None + Times in seconds of the first sample for each segment. An entry is None for a segment + without a start time. channel_ids : list An optional list of channel_ids. If None, linear channels are assumed """ @@ -55,7 +56,7 @@ def __init__(self, traces_list, sampling_frequency, t_starts=None, channel_ids=N if t_starts is not None: assert len(t_starts) == len(traces_list), "t_starts must be a list of same size than traces_list" - t_starts = [float(t_start) for t_start in t_starts] + t_starts = [None if t_start is None else float(t_start) for t_start in t_starts] self._serializability["json"] = False self._serializability["pickle"] = False @@ -142,8 +143,9 @@ class SharedMemoryRecording(BaseRecording): Note that the number of channels must be the same for all segments sampling_frequency : float The sampling frequency in Hz - t_starts : None or list of float - Times in seconds of the first sample for each segment + t_starts : None or list of float or None + Times in seconds of the first sample for each segment. An entry is None for a segment + without a start time. channel_ids : list An optional list of channel_ids. If None, linear channels are assumed main_shm_owner : bool, default: True @@ -153,11 +155,15 @@ class SharedMemoryRecording(BaseRecording): def __init__( self, shm_names, shape_list, dtype, sampling_frequency, channel_ids=None, t_starts=None, main_shm_owner=True ): + # Set these first: __del__ reads them, even when the constructor raises. + self.shms = [] + # this is important so that the main owner can unlink the share mem buffer : + self.main_shm_owner = main_shm_owner + assert len(shape_list) == len(shm_names), "Each shm_name in `shm_names` must have a shape in `shape_list`" assert all(shape_list[0][1] == shape[1] for shape in shape_list) # create traces from sharedmem names - self.shms = [] traces_list = [] for shm_name, shape in zip(shm_names, shape_list): shm = SharedMemory(shm_name, create=False) @@ -176,15 +182,12 @@ def __init__( if t_starts is not None: assert len(t_starts) == len(traces_list), "t_starts must be a list of same size as traces_list" - t_starts = [float(t_start) for t_start in t_starts] + t_starts = [None if t_start is None else float(t_start) for t_start in t_starts] self._serializability["memory"] = True self._serializability["json"] = False self._serializability["pickle"] = False - # this is important so that the main owner can unlink the share mem buffer : - self.main_shm_owner = main_shm_owner - for ( i, traces, diff --git a/src/spikeinterface/core/tests/test_baserecording.py b/src/spikeinterface/core/tests/test_baserecording.py index c20850a3e1..e1a6f8ce0e 100644 --- a/src/spikeinterface/core/tests/test_baserecording.py +++ b/src/spikeinterface/core/tests/test_baserecording.py @@ -536,6 +536,37 @@ def test_save_load_binary_with_time_vector(create_cache_folder, mp_context): assert not rec_saved_no_time_vector_par.has_time_vector(segment_index=0) +@pytest.mark.parametrize( + "save_kwargs", + [ + dict(format="binary"), + dict(format="memory", sharedmem=False), + dict(format="memory", sharedmem=True), + dict(format="zarr"), + ], + ids=["binary", "numpy", "sharedmem", "zarr"], +) +def test_save_with_t_start_on_some_segments(tmp_path, save_kwargs): + rec = generate_recording(num_channels=2, durations=[0.5, 0.2, 0.1], sampling_frequency=1000.0) + rec.shift_times(100.0, segment_index=1) + assert rec.get_segment_t_starts() == [None, 100.0, None] + + if save_kwargs["format"] in ("binary", "zarr"): + save_kwargs = dict(save_kwargs, folder=tmp_path / "saved") + saved = rec.save(**save_kwargs) + + # load() builds the recording again from what was saved, e.g. from binary.json for the binary format + for rec_to_check in (saved, load(saved.to_dict())): + assert rec_to_check.get_segment_t_starts() == [None, 100.0, None] + for segment_index in range(rec.get_num_segments()): + assert type(rec_to_check.get_start_time(segment_index)) is float + assert rec_to_check.get_start_time(segment_index) == rec.get_start_time(segment_index) + assert np.array_equal(rec_to_check.get_times(segment_index), rec.get_times(segment_index)) + assert np.array_equal( + rec_to_check.get_traces(segment_index=segment_index), rec.get_traces(segment_index=segment_index) + ) + + if __name__ == "__main__": import tempfile diff --git a/src/spikeinterface/core/tests/test_numpy_extractors.py b/src/spikeinterface/core/tests/test_numpy_extractors.py index 938fec8d71..0b4cdf95e9 100644 --- a/src/spikeinterface/core/tests/test_numpy_extractors.py +++ b/src/spikeinterface/core/tests/test_numpy_extractors.py @@ -1,3 +1,7 @@ +import gc +import sys +from multiprocessing.shared_memory import SharedMemory + import pytest import numpy as np @@ -60,6 +64,27 @@ def test_SharedMemoryRecording(): del rec +def test_SharedMemoryRecording_failed_init_releases_buffer(monkeypatch): + unraisable_exceptions = [] + monkeypatch.setattr(sys, "unraisablehook", unraisable_exceptions.append) + + shm = SharedMemory(create=True, size=160) + shm_name = shm.name + # fails before and after the buffer is attached + with pytest.raises(AssertionError): + SharedMemoryRecording([shm_name], [], "float64", 1000.0, main_shm_owner=True) + with pytest.raises(AssertionError): + SharedMemoryRecording([shm_name], [(10, 2)], "float64", 1000.0, t_starts=[0.0, 1.0], main_shm_owner=True) + # the recording is only deleted when the exception is released + gc.collect() + shm.close() + + assert unraisable_exceptions == [] + # __del__ of the main owner has unlinked the buffer + with pytest.raises(FileNotFoundError): + SharedMemory(shm_name, create=False) + + def test_NumpySorting(setup_NumpyRecording): sampling_frequency = 30000 diff --git a/src/spikeinterface/core/zarrextractors.py b/src/spikeinterface/core/zarrextractors.py index bfe70691ae..f11bc716fd 100644 --- a/src/spikeinterface/core/zarrextractors.py +++ b/src/spikeinterface/core/zarrextractors.py @@ -177,7 +177,7 @@ def __init__( if t_starts is None: t_start = None else: - t_start = t_starts[segment_index] + t_start = float(t_starts[segment_index]) if np.isnan(t_start): t_start = None time_kwargs["t_start"] = t_start From 1d206a32bbf779c8a016ea32796bb5c441169d29 Mon Sep 17 00:00:00 2001 From: AxelNoun <150222552+AxelNoun@users.noreply.github.com> Date: Tue, 10 Nov 2026 10:23:59 +0100 Subject: [PATCH 2/3] Remove the SharedMemoryRecording failed-init test Requested in review: the test is unrelated to the t_starts fix. --- .../core/tests/test_numpy_extractors.py | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/src/spikeinterface/core/tests/test_numpy_extractors.py b/src/spikeinterface/core/tests/test_numpy_extractors.py index 0b4cdf95e9..938fec8d71 100644 --- a/src/spikeinterface/core/tests/test_numpy_extractors.py +++ b/src/spikeinterface/core/tests/test_numpy_extractors.py @@ -1,7 +1,3 @@ -import gc -import sys -from multiprocessing.shared_memory import SharedMemory - import pytest import numpy as np @@ -64,27 +60,6 @@ def test_SharedMemoryRecording(): del rec -def test_SharedMemoryRecording_failed_init_releases_buffer(monkeypatch): - unraisable_exceptions = [] - monkeypatch.setattr(sys, "unraisablehook", unraisable_exceptions.append) - - shm = SharedMemory(create=True, size=160) - shm_name = shm.name - # fails before and after the buffer is attached - with pytest.raises(AssertionError): - SharedMemoryRecording([shm_name], [], "float64", 1000.0, main_shm_owner=True) - with pytest.raises(AssertionError): - SharedMemoryRecording([shm_name], [(10, 2)], "float64", 1000.0, t_starts=[0.0, 1.0], main_shm_owner=True) - # the recording is only deleted when the exception is released - gc.collect() - shm.close() - - assert unraisable_exceptions == [] - # __del__ of the main owner has unlinked the buffer - with pytest.raises(FileNotFoundError): - SharedMemory(shm_name, create=False) - - def test_NumpySorting(setup_NumpyRecording): sampling_frequency = 30000 From 2d584778d2c0fae3b6f57e5d1cf791d264fc0d8d Mon Sep 17 00:00:00 2001 From: Alessio Buccino Date: Tue, 10 Nov 2026 11:19:41 +0100 Subject: [PATCH 3/3] Apply suggestion from @alejoe91 --- src/spikeinterface/core/tests/test_baserecording.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/spikeinterface/core/tests/test_baserecording.py b/src/spikeinterface/core/tests/test_baserecording.py index e1a6f8ce0e..d5aa3c154b 100644 --- a/src/spikeinterface/core/tests/test_baserecording.py +++ b/src/spikeinterface/core/tests/test_baserecording.py @@ -551,8 +551,10 @@ def test_save_with_t_start_on_some_segments(tmp_path, save_kwargs): rec.shift_times(100.0, segment_index=1) assert rec.get_segment_t_starts() == [None, 100.0, None] - if save_kwargs["format"] in ("binary", "zarr"): + if save_kwargs["format"] == "binary": save_kwargs = dict(save_kwargs, folder=tmp_path / "saved") + elif save_kwargs["format"] == "zarr": + save_kwargs = dict(save_kwargs, folder=tmp_path / "saved.zarr") saved = rec.save(**save_kwargs) # load() builds the recording again from what was saved, e.g. from binary.json for the binary format