mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.66k stars 1.31k forks source link

ENH: error message is confusing when no ecg events are found by `create_ecg_epochs` #12201

Open eort opened 10 months ago

eort commented 10 months ago

Description of the problem

Calling mne.preprocessing.create_ecg_epochs() on data that contain an ECG channel that is malfunctioning in some way, such that no ecg events can be found, the function returns the somewhat misleading error message: TypeError: events should be a NumPy array of integers, got <class 'numpy.ndarray'>. This is because here: https://github.com/mne-tools/mne-python/blob/7b3e3c931914ee655486e7b8d5a5a30668ce136f/mne/preprocessing/ecg.py#L283 an empty numpy array without type specification is created in case no ECG events can be found.

This behavior is different from when an empty events array is passed to Epochs(), which results in : ValueError: zero-size array to reduction operation maximum which has no identity

Either way, both are a bit confusing when the error is essentially: "No ECG events can be found."

Steps to reproduce

import os
import mne
from mne.preprocessing import create_ecg_epochs

sample_data_folder = mne.datasets.sample.data_path()
sample_data_raw_file = os.path.join(sample_data_folder, 'MEG', 'sample',
                                    'sample_audvis_filt-0-40_raw.fif')
raw = mne.io.read_raw_fif(sample_data_raw_file)
# Here we'll crop to 60 seconds and drop gradiometer channels for speed
raw.crop(tmax=60.).pick_types(meg='mag', eeg=True, stim=True, eog=True)
raw.load_data()
raw.set_channel_types({'STI 004':'ecg'})
ecg = create_ecg_epochs(raw)

Expected results

An error message a la "No ECG events can be found."

Actual results

TypeError: events should be a NumPy array of integers, got <class 'numpy.ndarray'>

Additional information

Platform:         Linux-3.10.0-1160.49.1.el7.x86_64-x86_64-with-glibc2.17
Python:           3.9.16 | packaged by conda-forge | (main, Feb  1 2023, 21:39:03)  [GCC 11.3.0]
Executable:       /gpfs/project/projects/bpsydm/tools/pyEnvs/meg/bin/python
CPU:              x86_64: 24 cores
Memory:           187.4 GB

mne:              1.2.3
numpy:            1.23.5 {OpenBLAS 0.3.23 with 2 threads}
scipy:            1.11.1
matplotlib:       3.7.1 {backend=agg}

sklearn:          1.3.0
numba:            0.57.1
nibabel:          5.1.0
nilearn:          0.10.1
dipy:             1.7.0
openmeeg:         Not found
cupy:             Not found
pandas:           2.0.3
pyvista:          0.41.1 {OpenGL could not be initialized}
pyvistaqt:        0.0.0
ipyvtklink:       0.2.2
vtk:              9.2.6
qtpy:             2.3.1 {PyQt5=5.15.8}
ipympl:           Not found
pyqtgraph:        0.13.3
pooch:            v1.7.0

mne_bids:         0.11.1
mne_nirs:         Not found
mne_features:     Not found
mne_qt_browser:   0.5.2
mne_connectivity: Not found
mne_icalabel:     Not found
larsoner commented 10 months ago

It suggests that

  1. somewhere we do np.array(...) when we should do np.array(..., int)
  2. we should catch the empty events -- this might actually already happen once we np.array(..., int), but we'll have to see
eort commented 9 months ago

somewhere we do np.array(...) when we should do np.array(..., int)

Yes, exactly. I linked the location in the source code above.

this might actually already happen once we np.array(..., int), but we'll have to see

No, it doesn't unfortunately. You get a (different) ValueError instead.

we should catch the empty events

Yes, agreed. Right after the linked location above would be a good place. I haven't checked the entire preprocessing algorithm (eog, etc). Perhaps the same applies there.

ScreamingPigeon commented 9 months ago

I'm gonna take a look and try fixing this. Will keep you guys posted