MRG, BUG: Ensure channel entries are valid (#8416)

* BUG: Ensure channel entries are valid

* FIX: One more

Author: larsoner

doc/changes/0.21.inc

@@ -48,6 +48,8 @@ Bugs
 
 - Fix bug in :func:`mne.viz.plot_alignment` where ECoG and sEEG channels were not plotted and fNIRS channels were always plotted in the head coordinate frame by `Eric Larson`_ (:gh:`8393`)
 
+- Fix bug in :func:`mne.set_bipolar_reference` where ``ch_info`` could contain invalid channel information keys by `Eric Larson`_
+
 - Fix bug in :func:`mne.viz.plot_source_estimates`, which wouldn't accept the valid parameter value ``backend='pyvista'``, by `Guillaume Favelier`_
 
 - Attempting to remove baseline correction from preloaded `~mne.Epochs` will now raise an exception by `Richard Höchenberger`_ (:gh:`8435`)

mne/channels/tests/test_layout.py

@@ -39,10 +39,10 @@ def _get_test_info():
     loc = np.array([0., 0., 0., 1., 0., 0., 0., 1., 0., 0., 0., 1.],
                    dtype=np.float32)
     test_info['chs'] = [
-        {'cal': 1, 'ch_name': 'ICA 001', 'coil_type': 0, 'coord_Frame': 0,
+        {'cal': 1, 'ch_name': 'ICA 001', 'coil_type': 0, 'coord_frame': 0,
          'kind': 502, 'loc': loc.copy(), 'logno': 1, 'range': 1.0, 'scanno': 1,
          'unit': -1, 'unit_mul': 0},
-        {'cal': 1, 'ch_name': 'ICA 002', 'coil_type': 0, 'coord_Frame': 0,
+        {'cal': 1, 'ch_name': 'ICA 002', 'coil_type': 0, 'coord_frame': 0,
          'kind': 502, 'loc': loc.copy(), 'logno': 2, 'range': 1.0, 'scanno': 2,
          'unit': -1, 'unit_mul': 0},
         {'cal': 0.002142000012099743, 'ch_name': 'EOG 061', 'coil_type': 1,

mne/io/meas_info.py

@@ -64,6 +64,13 @@
 )
 
 
+_SCALAR_CH_KEYS = ('scanno', 'logno', 'kind', 'range', 'cal', 'coil_type',
+                   'unit', 'unit_mul', 'coord_frame')
+_ALL_CH_KEYS_SET = set(_SCALAR_CH_KEYS + ('loc', 'ch_name'))
+# XXX we need to require these except when doing simplify_info
+_MIN_CH_KEYS_SET = set(('kind', 'cal', 'unit', 'loc', 'ch_name'))
+
+
 def _get_valid_units():
     """Get valid units according to the International System of Units (SI).
 
@@ -191,6 +198,19 @@ def _format_trans(obj, key):
             obj[key] = Transform(t['from'], t['to'], t['trans'])
 
 
+def _check_ch_keys(ch, ci, name='info["chs"]', check_min=True):
+    ch_keys = set(ch)
+    bad = sorted(ch_keys.difference(_ALL_CH_KEYS_SET))
+    if bad:
+        raise KeyError(
+            f'key{_pl(bad)} errantly present for {name}[{ci}]: {bad}')
+    if check_min:
+        bad = sorted(_MIN_CH_KEYS_SET.difference(ch_keys))
+        if bad:
+            raise KeyError(
+                f'key{_pl(bad)} missing for {name}[{ci}]: {bad}',)
+
+
 # XXX Eventually this should be de-duplicated with the MNE-MATLAB stuff...
 class Info(dict, MontageMixin):
     """Measurement information.
@@ -733,15 +753,14 @@ def _check_consistency(self, prepend_error=''):
                 self[key] = float(self[key])
 
         # Ensure info['chs'] has immutable entries (copies much faster)
-        scalar_keys = ('unit_mul range cal kind coil_type unit '
-                       'coord_frame scanno logno').split()
         for ci, ch in enumerate(self['chs']):
+            _check_ch_keys(ch, ci)
             ch_name = ch['ch_name']
             if not isinstance(ch_name, str):
                 raise TypeError(
                     'Bad info: info["chs"][%d]["ch_name"] is not a string, '
                     'got type %s' % (ci, type(ch_name)))
-            for key in scalar_keys:
+            for key in _SCALAR_CH_KEYS:
                 val = ch.get(key, 1)
                 if not _is_numeric(val):
                     raise TypeError(

mne/io/reference.py

@@ -9,6 +9,7 @@
 from scipy import linalg
 
 from .constants import FIFF
+from .meas_info import _check_ch_keys
 from .proj import _has_eeg_average_ref_proj, make_eeg_average_ref_proj
 from .proj import setup_proj
 from .pick import pick_types, pick_channels, pick_channels_forward
@@ -469,15 +470,16 @@ def set_bipolar_reference(inst, anode, cathode, ch_name=None, ch_info=None,
 
     # Merge specified and anode channel information dictionaries
     new_chs = []
-    for an, ci in zip(anode, ch_info):
+    for ci, (an, ch) in enumerate(zip(anode, ch_info)):
+        _check_ch_keys(ch, ci, name='ch_info', check_min=False)
         an_idx = inst.ch_names.index(an)
         this_chs = deepcopy(inst.info['chs'][an_idx])
 
         # Set channel location and coil type
         this_chs['loc'] = np.zeros(12)
         this_chs['coil_type'] = FIFF.FIFFV_COIL_EEG_BIPOLAR
 
-        this_chs.update(ci)
+        this_chs.update(ch)
         new_chs.append(this_chs)
 
     if copy:

mne/io/tests/test_meas_info.py

@@ -485,6 +485,16 @@ def test_check_consistency():
         if key == 'ch_name':
             info['ch_names'][idx] = old
 
+    # bad channel entries
+    info2 = info.copy()
+    info2['chs'][0]['foo'] = 'bar'
+    with pytest.raises(KeyError, match='key errantly present'):
+        info2._check_consistency()
+    info2 = info.copy()
+    del info2['chs'][0]['loc']
+    with pytest.raises(KeyError, match='key missing'):
+        info2._check_consistency()
+
 
 def _test_anonymize_info(base_info):
     """Test that sensitive information can be anonymized."""

mne/io/tests/test_reference.py

@@ -319,9 +319,12 @@ def test_set_bipolar_reference():
     raw = read_raw_fif(fif_fname, preload=True)
     raw.apply_proj()
 
-    reref = set_bipolar_reference(raw, 'EEG 001', 'EEG 002', 'bipolar',
-                                  {'kind': FIFF.FIFFV_EOG_CH,
-                                   'extra': 'some extra value'})
+    ch_info = {'kind': FIFF.FIFFV_EOG_CH, 'extra': 'some extra value'}
+    with pytest.raises(KeyError, match='key errantly present'):
+        set_bipolar_reference(raw, 'EEG 001', 'EEG 002', 'bipolar', ch_info)
+    ch_info.pop('extra')
+    reref = set_bipolar_reference(
+        raw, 'EEG 001', 'EEG 002', 'bipolar', ch_info)
     assert (reref.info['custom_ref_applied'])
 
     # Compare result to a manual calculation
@@ -347,7 +350,6 @@ def test_set_bipolar_reference():
             assert_equal(bp_info[key], FIFF.FIFFV_EOG_CH)
         else:
             assert_equal(bp_info[key], an_info[key])
-    assert_equal(bp_info['extra'], 'some extra value')
 
     # Minimalist call
     reref = set_bipolar_reference(raw, 'EEG 001', 'EEG 002')
@@ -366,8 +368,8 @@ def test_set_bipolar_reference():
         ['EEG 001', 'EEG 003'],
         ['EEG 002', 'EEG 004'],
         ['bipolar1', 'bipolar2'],
-        [{'kind': FIFF.FIFFV_EOG_CH, 'extra': 'some extra value'},
-         {'kind': FIFF.FIFFV_EOG_CH, 'extra': 'some extra value'}],
+        [{'kind': FIFF.FIFFV_EOG_CH},
+         {'kind': FIFF.FIFFV_EOG_CH}],
     )
     a = raw.copy().pick_channels(['EEG 001', 'EEG 002', 'EEG 003', 'EEG 004'])
     a = np.array([a._data[0, :] - a._data[1, :],

mne/preprocessing/ica.py

@@ -932,8 +932,8 @@ def _export_info(self, info, container, add_channels):
             ch_info.append(dict(
                 ch_name=name, cal=1, logno=ii + 1,
                 coil_type=FIFF.FIFFV_COIL_NONE, kind=FIFF.FIFFV_MISC_CH,
-                coord_Frame=FIFF.FIFFV_COORD_UNKNOWN, unit=FIFF.FIFF_UNIT_NONE,
-                loc=np.array([0., 0., 0., 1.] * 3, dtype='f4'),
+                coord_frame=FIFF.FIFFV_COORD_UNKNOWN, unit=FIFF.FIFF_UNIT_NONE,
+                loc=np.zeros(12, dtype='f4'),
                 range=1.0, scanno=ii + 1, unit_mul=0))
 
         if add_channels is not None: