From f99b05219e44731accf5e4eac6c6f625dbbba7ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Fri, 19 Aug 2022 14:24:57 +0200 Subject: [PATCH] Make write_raw_bids() raise if an unsuitable extension is provided in the BIDSPath Some extensions / formats are only allowed for certain data types, e.g. .vhdr is (i)EEG-only. When passing a BIDSPath containing both datatype and extension to write_raw_bids(), in case of a mismatch we would previously simply silently replace the incorrect extension, effectively writing to a location (and file format) the user didn't expect. We now raise an exception in such situations. Fixes #1041 --- doc/whats_new.rst | 2 ++ mne_bids/tests/test_write.py | 48 ++++++++++++++++++------------------ mne_bids/write.py | 21 ++++++++++------ 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index afefb24b0..1e97765b4 100644 --- a/doc/whats_new.rst +++ b/doc/whats_new.rst @@ -96,6 +96,8 @@ Detailed list of changes - Whenever :func:`~mne_bids.read_raw_bids` encounters a channel type that currently doesn't translate into an appropriate MNE channel type, the channel type will now be set to ``'misc``. Previously, seemingly arbitrary channel types would be applied, e.g. ``'eeg'`` for GSR and temperature channels, by `Richard Höchenberger`_ (:gh:`1052`) +- If the :class:`~mne_bids.BIDSPath` passed to :func:`~mne_bids.write_raw_bids` contains an incompatible combination of ``datatype`` and ``extension``, we now raise an exception instead of silently overriding the user-specified extension, by `Richard Höchenberger`_ (:gh:`xxx`) + :doc:`Find out what was new in previous releases ` .. include:: authors.rst diff --git a/mne_bids/tests/test_write.py b/mne_bids/tests/test_write.py index 48837f725..d2af5e577 100644 --- a/mne_bids/tests/test_write.py +++ b/mne_bids/tests/test_write.py @@ -3156,32 +3156,32 @@ def test_format_conversion_overwrite(dir_name, format, fname, reader, write_raw_bids(**kwargs, overwrite=True) -@requires_version('mne', '0.22') -@pytest.mark.parametrize( - 'dir_name, format, fname, reader', test_converteeg_data) -@pytest.mark.filterwarnings( - warning_str['channel_unit_changed'], - warning_str['cnt_warning1'], - warning_str['cnt_warning2'], - warning_str['no_hand'], -) -def test_error_write_meg_as_eeg(dir_name, format, fname, reader, tmp_path): - """Test error writing as BrainVision EEG data for MEG.""" - bids_root = tmp_path / 'bids1' - data_path = op.join(testing.data_path(), dir_name) - raw_fname = op.join(data_path, fname) +def test_error_write_wrong_format(tmp_path): + """Don't allow using unsuitable extensions for a given datatype.""" + data_path = testing.data_path() + bids_root = tmp_path / 'bids_root' + bids_path = (_bids_path.copy() + .update(root=bids_root, datatype='eeg', extension='.vhdr')) - bids_path = _bids_path.copy().update(root=bids_root, datatype='eeg', - extension='.vhdr') - raw = reader(raw_fname) - kwargs = dict(raw=raw, format='auto', - bids_path=bids_path.update(datatype='meg')) + raw_path = data_path / 'MEG' / 'sample' / 'sample_audvis_trunc_raw.fif' + raw = _read_raw_fif(raw_path) + + # EEG data BIDSPath with an MEG-only extension + bids_path.update(datatype='eeg', suffix='eeg', extension='.fif') + with pytest.raises( + ValueError, + match=r'You requested to write eeg data with extension .fif' + ): + write_raw_bids(raw=raw, bids_path=bids_path) + + # MEG data BIDSPath with an EEG-only extension + bids_path.update(datatype='meg', suffix='meg', extension='.vhdr') + with pytest.raises( + ValueError, + match=r'You requested to write meg data with extension .vhdr' + ): + write_raw_bids(raw=raw, bids_path=bids_path) - # if we accidentally add MEG channels, then an error will occur - raw.set_channel_types({raw.info['ch_names'][0]: 'mag'}) - with pytest.raises(ValueError, match='Got file extension .*' - 'for MEG data'): - write_raw_bids(**kwargs) @pytest.mark.parametrize( diff --git a/mne_bids/write.py b/mne_bids/write.py index 7aab6c7bd..1f9c1640f 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -1524,6 +1524,20 @@ def write_raw_bids(raw, bids_path, events_data=None, event_id=None, '"bids_path.task = "' ) + if ( + (bids_path.datatype is not None and + bids_path.extension is not None) and + (bids_path.extension not in + ALLOWED_DATATYPE_EXTENSIONS[bids_path.datatype]) + ): + allowed_extensions = ALLOWED_DATATYPE_EXTENSIONS[bids_path.datatype] + raise ValueError( + f'You requested to write {bids_path.datatype} data with ' + f'extension {bids_path.extension} (as specified in bids_path), ' + f'but this is not allowed. ' + f'Please use one of: {", ".join(allowed_extensions)}' + ) + if events_data is not None and event_id is None: raise RuntimeError('You passed events_data, but no event_id ' 'dictionary. You need to pass both, or neither.') @@ -1819,13 +1833,6 @@ def write_raw_bids(raw, bids_path, events_data=None, event_id=None, 'Deactivate symbolic links by passing symlink=False to allow ' 'file format conversion.') - # check if there is an BIDS-unsupported MEG format - if bids_path.datatype == 'meg' and convert and not anonymize: - raise ValueError( - f"Got file extension {ext} for MEG data, " - f"expected one of " - f"{', '.join(sorted(ALLOWED_DATATYPE_EXTENSIONS['meg']))}") - if not convert: logger.info(f'Copying data files to {bids_path.fpath.name}')