MRG: TESTS: Remove RuntimeWarnings (#375)

* TESTS: Remove RuntimeWarnings

Make pytest fail on `RuntimeWarnings`, and only allow "legit",
explicitly specified warnings in tests.

At first, I wanted to "fix" or silence all warnings, but I quickly
realized that maybe this wasn't the best idea – also because I
ran into some weird interactions between the `warnings` module
and pytest being configured to on warnings, which cost me hours
to figure out.

That's why I've now decided to filter warnings that a test is
expected to raise via `pytest.mark.filterwarnings`. This allows us
not only to specify a warning message and category, but also
from which module to expect the warning, e.g. `mne_bids` or `mne`.
I've been very careful to check if all warnings that are silenced this
way are actually legit; there's only one warning in
`test_write.test_fif()` about which I'm unsure (marked via `XXX`).

* flake8

* Remove cruft

* TESTS: Add back accidentally deleted comment

* BF: Handle events with unknown onset or duration

Fixes an issue with MNE-Python 0.19:

`mne_bids.read._handle_events_reading()` would produce a warning when
event onsets or durations are `'n/a'`. This is because it replaces all
`'n/a'` strings with `np.nan` before creating  annotations. When this
Annotation object is then passed to `Raw.set_annotations()`, MNE calls
`Annotations.crop()`, which executes the following line:

```
out_of_bounds = (absolute_onset > tmax) | (absolute_offset < tmin)
```

Since there's now `np.nan` in `absolute_onset` and / or
`absolute_offset`, the desired behavior is unclear, and NumPy
will emit a `RuntimeWarning`. This warning, however, only occurs
with MNE-Python 0.19; with MNE `master`, the behavior of
`Annotations.crop()` has obviously changed.

To make things work without warning even with MNE-Python 0.19, I now
drop events with an unknown (`'n/a'` or `np.nan`) onset before
creating Annotations. And unknown durations are now simply assumed to be
zero.

* RF: Change order

* Use warning string dict

* Add changelog entry

* Remove XXX

doc/whats_new.rst

@@ -48,6 +48,7 @@ Bug
 - Fix :func:`read_raw_bids` to properly read in sidecar json even if a similarly named copy lives inside "derivatives/" sub-folder, by `Adam Li`_  (`#350 <https://github.com/mne-tools/mne-bids/pull/350>`_)
 - Fix :func:`read_raw_bids` to properly read in events.tsv sidecar if the 'trial_type' description has a "#" character in it by `Adam Li`_ (`#355 <https://github.com/mne-tools/mne-bids/pull/355>`_)
 - Avoid cases in which NumPy would raise a `FutureWarning` when populating TSV files, by `Richard Höchenberger`_ (`#372 <https://github.com/mne-tools/mne-bids/pull/372>`)
+- Remove events with an unknown onset, and assume unknown event durations to be zero, when reading BIDS data via :func:`read_raw_bids`, by `Richard Höchenberger`_ (`#375 <https://github.com/mne-tools/mne-bids/pull/375>`)
 
 API
 ~~~

mne_bids/commands/tests/test_cli.py

@@ -16,6 +16,10 @@
 subject_id = '01'
 task = 'testing'
 
+# Silence NumPy warnings
+# See https://stackoverflow.com/a/40846742
+pytestmark = pytest.mark.filterwarnings('ignore:numpy.ufunc size changed')
+
 
 def check_usage(module, force_help=False):
     """Ensure we print usage."""

mne_bids/read.py

@@ -143,11 +143,18 @@ def _handle_events_reading(events_fname, raw):
 
     # Deal with "n/a" strings before converting to float
     ons = [np.nan if on == 'n/a' else on for on in events_dict['onset']]
-    dus = [np.nan if du == 'n/a' else du for du in events_dict['duration']]
-
-    # Add Events to raw as annotations
+    dus = [0 if du == 'n/a' else du for du in events_dict['duration']]
     onsets = np.asarray(ons, dtype=float)
     durations = np.asarray(dus, dtype=float)
+
+    # Keep only events where onset is known
+    good_events_idx = ~np.isnan(onsets)
+    onsets = onsets[good_events_idx]
+    durations = durations[good_events_idx]
+    descriptions = descriptions[good_events_idx]
+    del good_events_idx
+
+    # Add Events to raw as annotations
     annot_from_events = mne.Annotations(onset=onsets,
                                         duration=durations,
                                         description=descriptions,

mne_bids/tests/test_read.py

@@ -49,6 +49,17 @@
 somato_raw_fname = op.join(somato_path, 'sub-01', 'meg',
                            'sub-01_task-somato_meg.fif')
 
+# Silence NumPy warnings
+# See https://stackoverflow.com/a/40846742
+pytestmark = pytest.mark.filterwarnings('ignore:numpy.ufunc size changed')
+
+warning_str = dict(
+    channel_unit_changed='ignore:The unit for chann*.:RuntimeWarning:mne',
+    meas_date_set_to_none="ignore:.*'meas_date' set to None:RuntimeWarning:"
+                          "mne",
+    nasion_not_found='ignore:.*nasion not found:RuntimeWarning:mne',
+)
+
 
 def test_read_raw():
     """Test the raw reading."""
@@ -71,6 +82,7 @@ def test_not_implemented():
 
 
 @requires_nibabel()
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_get_head_mri_trans():
     """Test getting a trans object from BIDS data."""
     import nibabel as nib
@@ -135,7 +147,7 @@ def test_handle_events_reading():
 
     # Create an arbitrary events.tsv file, to test we can deal with 'n/a'
     # make sure we can deal w/ "#" characters
-    events = {'onset': [11, 12, 13],
+    events = {'onset': [11, 12, 'n/a'],
               'duration': ['n/a', 'n/a', 'n/a'],
               'trial_type': ["rec start", "trial #1", "trial #2!"]
               }
@@ -147,6 +159,7 @@ def test_handle_events_reading():
     events, event_id = mne.events_from_annotations(raw)
 
 
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_line_freq_estimation():
     """Test estimating line frequency."""
     bids_root = _TempDir()
@@ -194,6 +207,7 @@ def test_line_freq_estimation():
     assert somato_raw.info['line_freq'] is None
 
 
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_handle_info_reading():
     """Test reading information from a BIDS sidecar.json file."""
     bids_root = _TempDir()
@@ -253,6 +267,8 @@ def test_handle_info_reading():
         raw = mne_bids.read_raw_bids(bids_fname, bids_root)
 
 
+@pytest.mark.filterwarnings(warning_str['nasion_not_found'])
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_handle_coords_reading():
     """Test reading coordinates from BIDS files."""
     bids_root = _TempDir()
@@ -300,6 +316,7 @@ def test_handle_coords_reading():
 
 
 @requires_nibabel()
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_get_head_mri_trans_ctf():
     """Test getting a trans object from BIDS data in CTF."""
     import nibabel as nib
@@ -332,6 +349,8 @@ def test_get_head_mri_trans_ctf():
     assert_almost_equal(trans['trans'], estimated_trans['trans'])
 
 
+@pytest.mark.filterwarnings(warning_str['meas_date_set_to_none'])
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_get_matched_empty_room():
     """Test reading of empty room data."""
     bids_root = _TempDir()
@@ -402,6 +421,7 @@ def test_get_matched_empty_room():
         get_matched_empty_room(bids_basename + '_meg.fif', bids_root)
 
 
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_read_raw_bids_pathlike():
     """Test that read_raw_bids() can handle a Path-like bids_root."""
     bids_root = _TempDir()

mne_bids/tests/test_write.py

@@ -53,6 +53,17 @@
     task=task)
 bids_basename_minimal = make_bids_basename(subject=subject_id, task=task)
 
+# Silence NumPy warnings
+# See https://stackoverflow.com/a/40846742
+pytestmark = pytest.mark.filterwarnings('ignore:numpy.ufunc size changed')
+
+warning_str = dict(
+    channel_unit_changed='ignore:The unit for chann*.:RuntimeWarning:mne',
+    meas_date_set_to_none="ignore:.*'meas_date' set to None:RuntimeWarning:"
+                          "mne",
+    nasion_not_found='ignore:.*nasion not found:RuntimeWarning:mne',
+    annotations_omitted='ignore:Omitted .* annot.*:RuntimeWarning:mne',
+)
 
 # WINDOWS issues:
 # the bids-validator development version does not work properly on Windows as
@@ -138,6 +149,8 @@ def test_get_anonymization_daysback():
 
 
 @requires_version('pybv', '0.2.0')
+@pytest.mark.filterwarnings(warning_str['annotations_omitted'])
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_fif(_bids_validate):
     """Test functionality of the write_raw_bids conversion for fif."""
     bids_root = _TempDir()
@@ -507,6 +520,7 @@ def test_kit(_bids_validate):
             overwrite=True)
 
 
+@pytest.mark.filterwarnings(warning_str['meas_date_set_to_none'])
 def test_ctf(_bids_validate):
     """Test functionality of the write_raw_bids conversion for CTF data."""
     bids_root = _TempDir()
@@ -542,6 +556,7 @@ def test_ctf(_bids_validate):
             get_anonymization_daysback(raw)
 
 
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_bti(_bids_validate):
     """Test functionality of the write_raw_bids conversion for BTi data."""
     bids_root = _TempDir()
@@ -579,7 +594,8 @@ def test_bti(_bids_validate):
 # XXX: vhdr test currently passes only on MNE master. Skip until next release.
 # see: https://github.com/mne-tools/mne-python/pull/6558
 @pytest.mark.skipif(LooseVersion(mne.__version__) < LooseVersion('0.19'),
-                    reason="requires mne 0.19.dev0 or higher")
+                    reason='requires mne 0.19.dev0 or higher')
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 def test_vhdr(_bids_validate):
     """Test write_raw_bids conversion for BrainVision data."""
     bids_root = _TempDir()
@@ -643,6 +659,7 @@ def test_vhdr(_bids_validate):
     _bids_validate(bids_root)
 
 
+@pytest.mark.filterwarnings(warning_str['nasion_not_found'])
 def test_edf(_bids_validate):
     """Test write_raw_bids conversion for European Data Format data."""
     bids_root = _TempDir()
@@ -793,6 +810,7 @@ def test_bdf(_bids_validate):
         _bids_validate(output_path)
 
 
+@pytest.mark.filterwarnings(warning_str['meas_date_set_to_none'])
 def test_set(_bids_validate):
     """Test write_raw_bids conversion for EEGLAB data."""
     # standalone .set file

setup.cfg

@@ -8,7 +8,6 @@ addopts =
     --ignore=doc
 filterwarnings =
     error
-    ignore::RuntimeWarning
     ignore::UserWarning
     ignore::ImportWarning