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()