BiocamRawIO: reshape flat datasets instead of gathering channel by channel

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

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:

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