[FIX] remove indexing along channel axis in AnalogSignalGap.load()#12357
[FIX] remove indexing along channel axis in AnalogSignalGap.load()#12357larsoner merged 6 commits intomne-tools:mainfrom
AnalogSignalGap.load()#12357Conversation
|
pip-pre will fail until pyvista/pyvista#5460 lands so you can ignore it |
larsoner
left a comment
There was a problem hiding this comment.
It would be great to have a test that would have caught this, something to keep in mind if you ever come across a suitably small dataset @KristijanArmeni !
|
Thanks, @larsoner! Can write a test at this point -- we already have 2 Neuralynx channels that simulate gaps, so I can reproduce this bug on the testing dataset. The code below fails on current main (but will pass on this branch): (3 durations < 0.005s hidden. Use -vv to show these durations.)
===================================== short test summary info ======================================
FAILED mne/io/neuralynx/tests/test_neuralynx.py::test_neuralynx_gaps - Failed: Failed to load data after picking one channel with gaps: all the input array dimensions...# test that channel selection works
raw = read_raw_neuralynx(
fname=testing_path,
preload=False,
exclude_fname_patterns=ignored_ncs_files,
)
raw.pick("LAHC2")
try:
raw.load_data()
except Exception as e:
pytest.fail(f"Failed to load data after picking one channel with gaps: {e}") |
Great, feel free to push a version of that here then! |
larsoner
left a comment
There was a problem hiding this comment.
Marking for merge when green, thanks in advance @KristijanArmeni !
Reference issue
Fixes #12356.
What does this implement/fix?
Removing indexing along channel dimension with
idxinAnalogSignalGap.load()on zero-arrays representing the gap data inself.signal(commit: 1aa66). These arrays are constructed on the fly using theidxvariable and hence have the appropriate channel dimension to begin with. In case of a single channel, slicing a 1-dimensional array withslice(1:2)slice yields a 0-dimensional array which breaks the concatenation downstream in l. 359.