From 826a7da411cfb1786b7165da4e7ee174b3bcd892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Fri, 27 Mar 2020 17:24:53 +0100 Subject: [PATCH] MRG: TESTS: Remove RuntimeWarnings (#375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 | 1 + mne_bids/commands/tests/test_cli.py | 4 ++++ mne_bids/read.py | 13 ++++++++++--- mne_bids/tests/test_read.py | 22 +++++++++++++++++++++- mne_bids/tests/test_write.py | 20 +++++++++++++++++++- setup.cfg | 1 - 6 files changed, 55 insertions(+), 6 deletions(-) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index 746298c01..fb5dc9c2f 100644 --- a/doc/whats_new.rst +++ b/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 `_) - 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 `_) - Avoid cases in which NumPy would raise a `FutureWarning` when populating TSV files, by `Richard Höchenberger`_ (`#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 `) API ~~~ diff --git a/mne_bids/commands/tests/test_cli.py b/mne_bids/commands/tests/test_cli.py index bc7fdec25..46819a176 100644 --- a/mne_bids/commands/tests/test_cli.py +++ b/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.""" diff --git a/mne_bids/read.py b/mne_bids/read.py index d1de9f252..c11bbdb3d 100644 --- a/mne_bids/read.py +++ b/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, diff --git a/mne_bids/tests/test_read.py b/mne_bids/tests/test_read.py index 58b7bb0e9..25d130d3e 100644 --- a/mne_bids/tests/test_read.py +++ b/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() diff --git a/mne_bids/tests/test_write.py b/mne_bids/tests/test_write.py index 718a50c16..a1a06a0b0 100644 --- a/mne_bids/tests/test_write.py +++ b/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 diff --git a/setup.cfg b/setup.cfg index f0c2cd69a..4376185dc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -8,7 +8,6 @@ addopts = --ignore=doc filterwarnings = error - ignore::RuntimeWarning ignore::UserWarning ignore::ImportWarning