From 1e7dcb3bf5467733bd75820728a220eed5679552 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Wed, 5 Aug 2026 05:05:54 -0700 Subject: [PATCH] BiocamRawIO: reshape flat datasets instead of gathering channel by channel The flat (frame-major) branch of _get_analogsignal_chunk expanded a slice with range(start or 0, stop or n_ch, step or 1), which silently mishandles any slice carrying a zero or negative bound, and then rebuilt the chunk with one strided gather per channel over an array that _read_function had already read fully into memory. Reshaping the flat array to (n_samples, n_channels) makes both file layouts share the same indexing expression, so numpy applies the caller's slice or index list directly and every slice form behaves as it does everywhere else in Neo. A frame range that runs past the end of the dataset now raises NeoReadWriteError naming the range rather than a numpy broadcast error. Fixes #1892 --- doc/source/authors.rst | 1 + neo/rawio/biocamrawio.py | 47 ++++++------- neo/test/rawiotest/test_biocamrawio.py | 95 ++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 27 deletions(-) diff --git a/doc/source/authors.rst b/doc/source/authors.rst index a2e15bdba..031523c60 100644 --- a/doc/source/authors.rst +++ b/doc/source/authors.rst @@ -103,6 +103,7 @@ and may not be the current affiliation of a contributor. * Reema Gupta [50] * Sai Asish Yamani [51] * Kevin Doran [52] +* Aditya Singh (github) 1. Centre de Recherche en Neuroscience de Lyon, CNRS UMR5292 - INSERM U1028 - Université Claude Bernard Lyon 1 2. Unité de Neuroscience, Information et Complexité, CNRS UPR 3293, Gif-sur-Yvette, France diff --git a/neo/rawio/biocamrawio.py b/neo/rawio/biocamrawio.py index d62cd6581..f27690b25 100644 --- a/neo/rawio/biocamrawio.py +++ b/neo/rawio/biocamrawio.py @@ -163,34 +163,27 @@ def _get_analogsignal_chunk(self, block_index, seg_index, i_start, i_stop, strea else: data = self._read_function(self._filehandle, i_start, i_stop, self._num_channels) - # older style data returns array of (n_samples, n_channels), should be a view + # Newer style data comes back as a flat array of length (n_samples * n_channels) + # laid out frame-major, i.e. every channel of frame 0, then every channel of + # frame 1, and so on. The read functions materialize it in memory already, so + # reshaping is a free view onto the same buffer and puts both file layouts on the + # same (n_samples, n_channels) footing. + if data.ndim == 1: + expected_size = (i_stop - i_start) * self._num_channels + if data.size != expected_size: + raise NeoReadWriteError( + f"Read {data.size} values for frames {i_start}:{i_stop} of {self._num_channels} " + f"channels but expected {expected_size}. The requested frame range is most likely " + "out of bounds." + ) + data = data.reshape(i_stop - i_start, self._num_channels) + + # older style data is already (n_samples, n_channels), should be a view # but if memory issues come up we should doublecheck out how the file is being stored - if data.ndim > 1: - if channel_indexes is None: - channel_indexes = slice(None) - sig_chunk = data[:, channel_indexes] - - # newer style data returns an initial flat array (n_samples * n_channels) - # we iterate through channels rather than slicing - # Due to the fact that Neo and SpikeInterface tend to prefer slices we need to add - # some careful checks around slicing of None in the case we need to iterate through - # channels. First check if None. Then check if slice and only if slice check that it is slice(None) - else: - if channel_indexes is None: - channel_indexes = [ch for ch in range(self._num_channels)] - elif isinstance(channel_indexes, slice): - start = channel_indexes.start or 0 - stop = channel_indexes.stop or self._num_channels - step = channel_indexes.step or 1 - channel_indexes = [ch for ch in range(start, stop, step)] - - sig_chunk = np.zeros((i_stop - i_start, len(channel_indexes)), dtype=data.dtype) - # iterate through channels to prevent loading all channels into memory which can cause - # memory exhaustion. See https://github.com/SpikeInterface/spikeinterface/issues/3303 - for index, channel_index in enumerate(channel_indexes): - sig_chunk[:, index] = data[channel_index :: self._num_channels] - - return sig_chunk + if channel_indexes is None: + channel_indexes = slice(None) + + return data[:, channel_indexes] def open_biocam_file_header(filename) -> dict: diff --git a/neo/test/rawiotest/test_biocamrawio.py b/neo/test/rawiotest/test_biocamrawio.py index 12ebf39ad..80db94de8 100644 --- a/neo/test/rawiotest/test_biocamrawio.py +++ b/neo/test/rawiotest/test_biocamrawio.py @@ -2,12 +2,14 @@ Tests of neo.rawio.BiocamRawIO """ +import json import unittest import pytest import numpy as np import h5py +from neo.core import NeoReadWriteError from neo.rawio.biocamrawio import BiocamRawIO from neo.test.rawiotest.common_rawio_test import BaseTestRawIO @@ -61,5 +63,98 @@ def test_biocamrawio_gain(tmp_path): assert expected_gain == pytest.approx(gain) +def _write_minimal_brw4(path, n_ch, n_frames): + """Write a minimal uncompressed BRW 4.x file whose ``Raw`` dataset is flat and frame-major. + + Returns the reference (n_frames, n_ch) view of the samples that were written. + """ + experiment_settings = { + "ValueConverter": { + "MaxAnalogValue": 4125.0, + "MinAnalogValue": -4125.0, + "MaxDigitalValue": 4096, + "MinDigitalValue": 0, + "ScaleFactor": 1.0, + }, + "TimeConverter": {"FrameRate": 19753.775}, + } + raw = np.arange(n_ch * n_frames, dtype=np.uint16) + with h5py.File(path, "w") as f: + f.create_dataset("ExperimentSettings", data=[json.dumps(experiment_settings).encode()]) + f.create_dataset("TOC", data=np.array([[0, n_frames]], dtype=np.int64)) + well = f.create_group("Well_A1") + well.create_dataset("StoredChIdxs", data=np.arange(n_ch, dtype=np.int32)) + well.create_dataset("Raw", data=raw) + return raw.reshape(n_frames, n_ch) + + +@pytest.mark.parametrize( + "channel_indexes", + [ + None, + slice(None), + slice(0, 4), + slice(2, None), + slice(None, 3), + slice(None, None, 2), + slice(0, 0), + slice(0, -1), + slice(-2, None), + slice(None, None, -1), + [3, 1, 15], + np.array([0, 5]), + ], +) +def test_biocamrawio_flat_layout_channel_selection(tmp_path, channel_indexes): + """Selecting channels from a flat (frame-major) Biocam dataset must match a plain reshape. + + A test case from Issue #1892 (https://github.com/NeuralEnsemble/python-neo/issues/1892). + The flat branch used to expand a slice with ``range(start or 0, stop or n_ch, step or 1)``, + which silently mishandles every slice carrying a negative or zero bound: ``slice(0, -1)`` + and ``slice(None, None, -1)`` returned no channels, ``slice(0, 0)`` returned all of them, + and ``slice(-2, None)`` returned more columns than the file has channels. + """ + n_ch, n_frames = 16, 40 + path = tmp_path / "minimal_v4.brw" + reference = _write_minimal_brw4(path, n_ch, n_frames) + + reader = BiocamRawIO(filename=path) + reader.parse_header() + + i_start, i_stop = 5, 25 + chunk = reader.get_analogsignal_chunk( + block_index=0, + seg_index=0, + i_start=i_start, + i_stop=i_stop, + stream_index=0, + channel_indexes=channel_indexes, + ) + + expected = reference[i_start:i_stop][:, slice(None) if channel_indexes is None else channel_indexes] + assert chunk.shape == expected.shape + assert np.array_equal(chunk, expected) + + +def test_biocamrawio_flat_layout_out_of_bounds(tmp_path): + """A frame range past the end of a flat dataset raises instead of silently mis-shaping.""" + n_ch, n_frames = 16, 40 + path = tmp_path / "minimal_v4.brw" + _write_minimal_brw4(path, n_ch, n_frames) + + reader = BiocamRawIO(filename=path) + reader.parse_header() + + with pytest.raises(NeoReadWriteError): + reader.get_analogsignal_chunk( + block_index=0, + seg_index=0, + i_start=0, + i_stop=n_frames + 3, + stream_index=0, + channel_indexes=None, + ) + + if __name__ == "__main__": unittest.main()