[MRG] Fixing quickstart doc bug, write_raw_bids doc bug, and coordsys…

…tem.json overwriting error (#675)

* Fixing doc bugs.

* Adding tests.

* Fixing coverage.

* Adding whatsnew.

* Update doc/whats_new.rst

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

* Update mne_bids/dig.py

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

* Update mne_bids/dig.py

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

* Update mne_bids/dig.py

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

* Fixing runtimeerror string.

* Add test to eeg/ieeg as well.

* Fixing unit test.

* Fixing whatsnew.

* Fixing whatsnew.

* Fixing oneliner.

* Fixing unit tests.

* Update mne_bids/dig.py

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>

doc/use.rst

@@ -15,7 +15,7 @@ Python
     >>> from mne_bids import BIDSPath, write_raw_bids
     >>> raw = mne.io.read_raw_fif('my_old_file.fif')
     >>> bids_path = BIDSPath(subject='01', session='01, run='05',
-                             datatype='meg', bids_root='./bids_dataset')
+                             datatype='meg', root='./bids_dataset')
     >>> write_raw_bids(raw, bids_path=bids_path)
 
 Command Line Interface

doc/whats_new.rst

@@ -48,6 +48,7 @@ Bug fixes
 
 - Fix writing MEGIN Triux files, by `Alexandre Gramfort`_ (:gh:`674`)
 - Anonymization of EDF files in :func:`write_raw_bids` will now convert recording date to ``01-01-1985 00:00:00`` if anonymization takes place, while setting the recording date in the ``scans.tsv`` file to the anonymized date, thus making the file EDF/EDFBrowser compliant, by `Adam Li`_ (:gh:`669`)
+- :func:`mne_bids.write_raw_bids` will not overwrite an existing ``coordsystem.json`` anymore, unless explicitly requested, by `Adam Li`_ (:gh:`675`)
 
 :doc:`Find out what was new in previous releases <whats_new_previous_releases>`
 

mne_bids/dig.py

@@ -5,6 +5,7 @@
 # License: BSD (3-clause)
 import json
 from collections import OrderedDict
+from pathlib import Path
 
 import mne
 import numpy as np
@@ -206,7 +207,23 @@ def _electrodes_tsv(raw, fname, datatype, overwrite=False, verbose=True):
     if hasattr(raw, 'impedances'):
         data['impedance'] = _get_impedances(raw, names)
 
-    _write_tsv(fname, data, overwrite=overwrite, verbose=verbose)
+    # note that any coordsystem.json file shared within sessions
+    # will be the same across all runs (currently). So
+    # overwrite is set to True always
+    # XXX: improve later when BIDS is updated
+    # check that there already exists a coordsystem.json
+    if Path(fname).exists() and not overwrite:
+        electrodes_tsv = _from_tsv(fname)
+
+        # cast values to str to make equality check work
+        if any([list(map(str, vals1)) != list(vals2) for vals1, vals2 in
+                zip(data.values(), electrodes_tsv.values())]):
+            raise RuntimeError(
+                f'Trying to write electrodes.tsv, but it already '
+                f'exists at {fname} and the contents do not match. '
+                f'You must differentiate this electrodes.tsv file '
+                f'from the existing one, or set "overwrite" to True.')
+    _write_tsv(fname, data, overwrite=True, verbose=verbose)
 
 
 def _coordsystem_json(*, raw, unit, hpi_coord_system, sensor_coord_system,
@@ -290,7 +307,21 @@ def _coordsystem_json(*, raw, unit, hpi_coord_system, sensor_coord_system,
             'iEEGCoordinateUnits': unit,  # m (MNE), mm, cm , or pixels
         }
 
-    _write_json(fname, fid_json, overwrite, verbose)
+    # note that any coordsystem.json file shared within sessions
+    # will be the same across all runs (currently). So
+    # overwrite is set to True always
+    # XXX: improve later when BIDS is updated
+    # check that there already exists a coordsystem.json
+    if Path(fname).exists() and not overwrite:
+        with open(fname, 'r', encoding='utf-8-sig') as fin:
+            coordsystem_dict = json.load(fin)
+        if fid_json != coordsystem_dict:
+            raise RuntimeError(
+                f'Trying to write coordsystem.json, but it already '
+                f'exists at {fname} and the contents do not match. '
+                f'You must differentiate this coordsystem.json file '
+                f'from the existing one, or set "overwrite" to True.')
+    _write_json(fname, fid_json, overwrite=True, verbose=verbose)
 
 
 def _write_dig_bids(bids_path, raw, overwrite=False, verbose=True):

mne_bids/tests/test_write.py

@@ -43,7 +43,7 @@
                       mark_bad_channels, write_meg_calibration,
                       write_meg_crosstalk, get_entities_from_fname)
 from mne_bids.utils import (_stamp_to_dt, _get_anonymization_daysback,
-                            get_anonymization_daysback)
+                            get_anonymization_daysback, _write_json)
 from mne_bids.tsv_handler import _from_tsv, _to_tsv
 from mne_bids.sidecar_updates import _update_sidecar
 from mne_bids.path import _find_matching_sidecar
@@ -2127,6 +2127,146 @@ def test_undescribed_events(_bids_validate, drop_undescribed_events):
     _bids_validate(bids_root)
 
 
+@pytest.mark.parametrize(
+    'dir_name, fname, reader, datatype, coord_frame', [
+        ('EDF', 'test_reduced.edf', _read_raw_edf, 'ieeg', 'mni_tal'),
+        ('EDF', 'test_reduced.edf', _read_raw_edf, 'ieeg', 'fs_tal'),
+        ('EDF', 'test_reduced.edf', _read_raw_edf, 'ieeg', 'unknown'),
+        ('EDF', 'test_reduced.edf', _read_raw_edf, 'eeg', 'head'),
+        ('EDF', 'test_reduced.edf', _read_raw_edf, 'eeg', 'mri'),
+        ('EDF', 'test_reduced.edf', _read_raw_edf, 'eeg', 'unknown'),
+        ('CTF', 'testdata_ctf.ds', _read_raw_ctf, 'meg', ''),
+        ('MEG', 'sample/sample_audvis_trunc_raw.fif', _read_raw_fif, 'meg', ''),  # noqa
+    ]
+)
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
+@pytest.mark.filterwarnings(warning_str['encountered_data_in'])
+@pytest.mark.filterwarnings(warning_str['nasion_not_found'])
+def test_coordsystem_json_compliance(
+        dir_name, fname, reader, datatype, coord_frame):
+    """Tests that coordsystem.json contents are written correctly.
+
+    Tests multiple manufacturer data formats and MEG, EEG, and iEEG.
+
+    TODO: Fix coordinatesystemdescription for iEEG.
+    Currently, iEEG coordinate system descriptions are not written
+    correctly.
+    """
+    bids_root = _TempDir()
+    data_path = op.join(testing.data_path(), dir_name)
+    raw_fname = op.join(data_path, fname)
+
+    # the BIDS path for test datasets to get written to
+    bids_path = _bids_path.copy().update(root=bids_root,
+                                         datatype=datatype)
+
+    raw = reader(raw_fname)
+
+    # clean all events for this test
+    kwargs = dict(raw=raw, bids_path=bids_path, overwrite=True,
+                  verbose=False)
+
+    if datatype == 'eeg':
+        raw.set_channel_types({ch: 'eeg' for ch in raw.ch_names})
+        landmarks = dict(nasion=[1, 0, 0],
+                         lpa=[0, 1, 0],
+                         rpa=[0, 0, 1])
+    elif datatype == 'ieeg':
+        raw.set_channel_types({ch: 'seeg' for ch in raw.ch_names})
+
+        # XXX: setting landmarks for ieeg montage for some reason
+        # transforms coord_frame -> 'CapTrak'. Possible issue in mne
+        landmarks = dict()
+
+    if datatype != 'meg':
+        # alter some channels manually with electrodes to write
+        ch_names = raw.ch_names
+        elec_locs = np.random.random((len(ch_names), 3)).tolist()
+        ch_pos = dict(zip(ch_names, elec_locs))
+        montage = mne.channels.make_dig_montage(ch_pos=ch_pos,
+                                                coord_frame=coord_frame,
+                                                **landmarks)
+        raw.set_montage(montage)
+
+    # write to BIDS and then check the coordsystem files
+    bids_output_path = write_raw_bids(**kwargs)
+    coordsystem_fname = _find_matching_sidecar(bids_output_path,
+                                               suffix='coordsystem',
+                                               extension='.json')
+    with open(coordsystem_fname, 'r', encoding='utf-8') as fin:
+        coordsystem_json = json.load(fin)
+
+    # writing twice should work as long as the coordsystem
+    # contents have not changed
+    write_raw_bids(raw=raw, bids_path=bids_path.copy().update(run='02'),
+                   overwrite=False, verbose=False)
+
+    datatype_ = {'meg': 'MEG', 'eeg': 'EEG', 'ieeg': 'iEEG'}[datatype]
+    # if there is a change in the underlying
+    # coordsystem.json file, then an error will occur.
+    # upon changing coordsystem contents, and overwrite not True
+    # this will fail
+    new_coordsystem_json = coordsystem_json.copy()
+    new_coordsystem_json[f'{datatype_}CoordinateSystem'] = 'blah'
+    _write_json(coordsystem_fname, new_coordsystem_json, overwrite=True)
+    with pytest.raises(RuntimeError,
+                       match='Trying to write coordsystem.json, '
+                             'but it already exists'):
+        write_raw_bids(raw=raw, bids_path=bids_path.copy().update(run='03'),
+                       overwrite=False, verbose=False)
+    _write_json(coordsystem_fname, coordsystem_json, overwrite=True)
+
+    if datatype != 'meg':
+        electrodes_fname = _find_matching_sidecar(bids_output_path,
+                                                  suffix='electrodes',
+                                                  extension='.tsv')
+        elecs_tsv = _from_tsv(electrodes_fname)
+
+        # electrodes.tsv file, then an error will occur.
+        # upon changing electrodes contents, and overwrite not True
+        # this will fail
+        new_elecs_tsv = elecs_tsv.copy()
+        new_elecs_tsv['name'][0] = 'blah'
+        _to_tsv(new_elecs_tsv, electrodes_fname)
+        with pytest.raises(
+                RuntimeError, match='Trying to write electrodes.tsv, '
+                                    'but it already exists'):
+            write_raw_bids(
+                raw=raw, bids_path=bids_path.copy().update(run='04'),
+                overwrite=False, verbose=False)
+
+    # perform checks on the coordsystem.json file itself
+    if datatype == 'eeg' and coord_frame == 'head':
+        assert coordsystem_json['EEGCoordinateSystem'] == 'CapTrak'
+        assert coordsystem_json['EEGCoordinateSystemDescription'] == \
+            COORD_FRAME_DESCRIPTIONS['captrak']
+    elif datatype == 'eeg' and coord_frame == 'unknown':
+        assert coordsystem_json['EEGCoordinateSystem'] == 'CapTrak'
+        assert coordsystem_json['EEGCoordinateSystemDescription'] == \
+            COORD_FRAME_DESCRIPTIONS['captrak']
+    elif datatype == 'ieeg' and coord_frame == 'mni_tal':
+        assert 'space-mni' in coordsystem_fname
+        assert coordsystem_json['iEEGCoordinateSystem'] == 'Other'
+        # assert coordsystem_json['iEEGCoordinateSystemDescription'] == \
+        #     COORD_FRAME_DESCRIPTIONS['mni_tal']
+    elif datatype == 'ieeg' and coord_frame == 'fs_tal':
+        assert 'space-fs' in coordsystem_fname
+        assert coordsystem_json['iEEGCoordinateSystem'] == 'Other'
+        # assert coordsystem_json['iEEGCoordinateSystemDescription'] == \
+        #     COORD_FRAME_DESCRIPTIONS['fs_tal']
+    elif datatype == 'ieeg' and coord_frame == 'unknown':
+        assert coordsystem_json['iEEGCoordinateSystem'] == 'Other'
+        # assert coordsystem_json['iEEGCoordinateSystemDescription'] == 'n/a'
+    elif datatype == 'meg' and dir_name == 'CTF':
+        assert coordsystem_json['MEGCoordinateSystem'] == 'CTF'
+        assert coordsystem_json['MEGCoordinateSystemDescription'] == \
+            COORD_FRAME_DESCRIPTIONS['ctf']
+    elif datatype == 'meg' and dir_name == 'MEG':
+        assert coordsystem_json['MEGCoordinateSystem'] == 'ElektaNeuromag'
+        assert coordsystem_json['MEGCoordinateSystemDescription'] == \
+            COORD_FRAME_DESCRIPTIONS['elektaneuromag']
+
+
 @pytest.mark.parametrize(
     'subject, dir_name, fname, reader', [
         ('01', 'EDF', 'test_reduced.edf', _read_raw_edf),

mne_bids/write.py

@@ -933,14 +933,15 @@ def write_raw_bids(raw, bids_path, events_data=None,
         Example::
 
             bids_path = BIDSPath(subject='01', session='01', task='testing',
-                                 acquisition='01', run='01', root='/data/BIDS')
+                                 acquisition='01', run='01', datatype='meg',
+                                 root='/data/BIDS')
 
         This will write the following files in the correct subfolder ``root``::
 
             sub-01_ses-01_task-testing_acq-01_run-01_meg.fif
             sub-01_ses-01_task-testing_acq-01_run-01_meg.json
             sub-01_ses-01_task-testing_acq-01_run-01_channels.tsv
-            sub-01_ses-01_task-testing_acq-01_run-01_coordsystem.json
+            sub-01_ses-01_acq-01_coordsystem.json
 
         and the following one if ``events_data`` is not ``None``::