Fix BTi handling in mne-bids (#1160)

* fix bti copyfile

* remove pragma, this is covered

* do not rename bti files, but warn if they are weird

* update whatsnew

* skip tests if mne not >=1.5

* fix numbers for versions

* add option to skip circle [skip circle] [skip ci]

.circleci/config.yml

@@ -1,13 +1,26 @@
 # See: https://circleci.com/blog/deploying-documentation-to-github-pages-with-continuous-integration/
 version: 2.1
 
+_check_skip: &check_skip
+  name: Check-skip
+  command: |
+    set -e
+    export COMMIT_MESSAGE=$(git log --format=oneline -n 1);
+    if [[ -v CIRCLE_PULL_REQUEST ]] && ([[ "$COMMIT_MESSAGE" == *"[skip circle]"* ]] || [[ "$COMMIT_MESSAGE" == *"[circle skip]"* ]]); then
+      echo "Skip detected, exiting job ${CIRCLE_JOB} for PR ${CIRCLE_PULL_REQUEST}."
+      circleci-agent step halt;
+    fi
+
 jobs:
   docs-build:
     machine:
         image: ubuntu-2004:202111-01
     steps:
         - checkout
 
+        - run:
+            <<: *check_skip
+
         # restore cache from last build. Unless __init__.py has changed since then
         - restore_cache:
             keys:

doc/whats_new.rst

@@ -50,6 +50,7 @@ Detailed list of changes
 
 - When writing events, we now also create an ``*_events.json`` file in addition to ``*_events.tsv``. This ensures compatibility with the upcoming release of BIDS 1.9, by `Richard Höchenberger`_ (:gh:`1132`)
 - We silenced warnings about missing ``events.tsv`` files when reading empty-room or resting-state data, by `Richard Höchenberger`_ (:gh:`1133`)
+- BTi 'processed data files' (pdf) will no longer be renamed to ``c,rf*`` by default. Instead, they will be copied over without name changes. When reading BTi data via ``mne-bids``, more informative log and error messages will help choosing the right course of action, by `Stefan Appelhoff`_ (:gh:`1160`)
 
 🛠 Requirements
 ^^^^^^^^^^^^^^^
@@ -65,7 +66,7 @@ Detailed list of changes
 - Improve compatibility with latest MNE-Python, by `Eric Larson`_ (:gh:`1128`)
 - Working with :class:`~mne_bids.BIDSPath` would sometimes inadvertently create new directories, contaminating the BIDS dataset, by `Richard Höchenberger`_ (:gh:`1139`)
 - Fix thrown error if the ``BIDSVersion`` defined in ``dataset_description.json`` file does not match the MNE-BIDS compliant ``BIDSVersion``, ensuring backwards compatibility across BIDS complient tools, by `Ford McDonald`_ (:gh:`1147`)
-- Copying BTI files without a headshape file will no longer raise an error, the file will simply be copied, and the missing headshape file will be ignored, by `Stefan Appelhoff`_ (:gh:`1158`)
+- Copying BTi files without a headshape file will no longer raise an error, the file will simply be copied, and the missing headshape file will be ignored, by `Stefan Appelhoff`_ (:gh:`1158`)
 
 :doc:`Find out what was new in previous releases <whats_new_previous_releases>`
 

doc/whats_new_previous_releases.rst

@@ -781,7 +781,7 @@ Bug fixes
 - Fix :func:`write_raw_bids` when ``info['dig']`` is ``None``, by `Alexandre Gramfort`_ (:gh:`452`)
 - :func:`mne_bids.write_raw_bids` now applies ``verbose`` to the functions that read events, by `Evgenii Kalenkovich`_ (:gh:`453`)
 - Fix ``raw_to_bids`` CLI tool to work with non-FIFF files, by `Austin Hurst`_ (:gh:`456`)
-- Fix :func:`mne_bids.write_raw_bids` to output BTI and CTF data in the ``scans.tsv`` according to the BIDS specification, by `Adam Li`_ (:gh:`465`)
+- Fix :func:`mne_bids.write_raw_bids` to output BTi and CTF data in the ``scans.tsv`` according to the BIDS specification, by `Adam Li`_ (:gh:`465`)
 - :func:`mne_bids.read_raw_bids` now populates the list of bad channels based on ``*_channels.tsv`` if (and only if) a ``status`` column is present, ignoring similar metadata stored in raw file (which will still be used if **no** ``status`` column is present in ``*_channels.tsv``), by `Richard Höchenberger`_ (:gh:`499`)
 - Ensure that ``Raw.info['bads']`` returned by :func:`mne_bids.read_raw_bids` is always a list, by `Richard Höchenberger`_ (:gh:`501`)
 - :func:`mne_bids.write_raw_bids` now ensures that **all** parts of the :class:`mne.io.Raw` instance stay in sync when using anonymization to shift dates, e.g. ``raw.annotations``, by `Richard Höchenberger`_ (:gh:`504`)

mne_bids/copyfiles.py

@@ -16,18 +16,19 @@
 # License: BSD-3-Clause
 import os
 import os.path as op
+from pathlib import Path
 import re
 import shutil as sh
-from pathlib import Path
-
-from scipy.io import loadmat, savemat
 
+import mne
 from mne.io import read_raw_brainvision, read_raw_edf, read_raw_bdf, anonymize_info
 from mne.utils import logger, verbose
+from mne.fixes import _compare_version
+import numpy as np
+from scipy.io import loadmat, savemat
 
 from mne_bids.path import BIDSPath, _parse_ext, _mkdir_p
 from mne_bids.utils import _get_mrk_meas_date, _check_anonymize, warn
-import numpy as np
 
 
 def _copytree(src, dst, **kwargs):
@@ -616,13 +617,28 @@ def copyfile_bti(raw, dest):
     copyfile_kit
 
     """
-    pdf_fname = "c,rfDC"
-    if raw.info["highpass"] is not None:
-        pdf_fname = "c,rf%0.1fHz" % raw.info["highpass"]
-    sh.copyfile(raw._init_kwargs["pdf_fname"], op.join(dest, pdf_fname))
-    sh.copyfile(raw._init_kwargs["config_fname"], op.join(dest, "config"))
-
-    # If no headshape file present, cannot copy it
-    hs_file = raw._init_kwargs.get("head_shape_fname")
-    if hs_file is not None:  # pragma: no cover
-        sh.copyfile(hs_file, op.join(dest, "hs_file"))
+    # XXX: remove after support for mne<1.5 is dropped
+    # BTi operations were unreliable in mne-bids prior to mne 1.5 / mne-bids 0.13
+    need_mne = "1.4.2"
+    if not _compare_version(mne.__version__, ">", need_mne):  # pragma: no cover
+        raise ImportError(
+            f"mne > {need_mne} is required for BTi operations, got {mne.__version__}"
+        )
+
+    os.makedirs(dest, exist_ok=True)
+    for key, val in (
+        ("pdf_fname", None),
+        ("config_fname", "config"),
+        ("head_shape_fname", "hs_file"),
+    ):
+        keyfile = raw._raw_extras[0].get(key)
+
+        # Keep name of pdf file
+        if key == "pdf_fname":
+            val = op.basename(keyfile)
+
+        # If no headshape file present, cannot copy it
+        if key == "head_shape_fname" and keyfile is None:
+            continue
+
+        sh.copyfile(keyfile, op.join(dest, val))

mne_bids/path.py

@@ -1320,7 +1320,7 @@ def _get_matching_bidspaths_from_filesystem(bids_path):
         subject=sub, session=ses, datatype=datatype, root=bids_root
     ).directory
 
-    # For BTI data, just return the directory with a '.pdf' extension
+    # For BTi data, just return the directory with a '.pdf' extension
     # to facilitate reading in mne-bids
     bti_dir = op.join(data_dir, f"{basename}")
     if op.isdir(bti_dir):

mne_bids/read.py

@@ -61,8 +61,8 @@ def _read_raw(
     # BTi systems
     elif ext == ".pdf":
         raw = io.read_raw_bti(
-            pdf_fname=str(raw_path),  # FIXME MNE should accept Path!
-            config_fname=str(config_path),  # FIXME MNE should accept Path!
+            pdf_fname=raw_path,
+            config_fname=config_path,
             head_shape_fname=hsp,
             preload=False,
             **kwargs,
@@ -280,7 +280,7 @@ def _handle_scans_reading(scans_fname, raw, bids_path):
     fname = bids_path.fpath.name
 
     if fname.endswith(".pdf"):
-        # for BTI files, the scan is an entire directory
+        # for BTi files, the scan is an entire directory
         fname = fname.split(".")[0]
 
     # get the row corresponding to the file
@@ -759,7 +759,29 @@ def read_raw_bids(bids_path, extra_params=None, verbose=None):
 
     if bids_path.fpath.suffix == ".pdf":
         bids_raw_folder = bids_path.directory / f"{bids_path.basename}"
-        raw_path = list(bids_raw_folder.glob("c,rf*"))[0]
+
+        # try to find the processed data file ("pdf")
+        # see: https://www.fieldtriptoolbox.org/getting_started/bti/
+        bti_pdf_patterns = ["0", "c,rf*", "hc,rf*", "e,rf*"]
+        pdf_list = []
+        for pattern in bti_pdf_patterns:
+            pdf_list += sorted(bids_raw_folder.glob(pattern))
+
+        if len(pdf_list) == 0:
+            raise RuntimeError(
+                "Cannot find BTi 'processed data file' (pdf). Please open an issue on "
+                "the mne-bids repository to discuss with the developers:\n\n"
+                "https://github.com/mne-tools/mne-bids/issues/new/choose\n\n"
+                f"No matches for following patterns:\n\n{bti_pdf_patterns}\n\n"
+                f"In: {bids_raw_folder}"
+            )
+        elif len(pdf_list) > 1:  # pragma: no cover
+            logger.warn(
+                "Found more than one BTi 'processed data file' (pdf). "
+                f"Picking:\n\n    {pdf_list[0]}\n\nout of the options:\n\n"
+                f"{pdf_list}\n\n"
+            )
+        raw_path = pdf_list[0]
         config_path = bids_raw_folder / "config"
     else:
         raw_path = bids_path.fpath

mne_bids/tests/test_copyfiles.py

@@ -21,6 +21,7 @@
     copyfile_edf,
     copyfile_eeglab,
     copyfile_kit,
+    copyfile_bti,
 )
 
 testing_path = testing.data_path(download=False)
@@ -299,3 +300,18 @@ def test_copyfile_kit(tmp_path):
             root=output_path,
         )
         assert op.exists(hsp_fname)
+
+
+@pytest.mark.parametrize("dataset", ("4Dsim", "erm_HFH"))
+@testing.requires_testing_data
+def test_copyfile_bti(tmp_path, dataset):
+    """Test copying and renaming BTi files to a new location."""
+    pytest.importorskip("mne", "1.5.0.dev")  # XXX: remove when mne<1.5 is dropped
+    pdf_fname = testing_path / "BTi" / dataset / "c,rfDC"
+    kwargs = dict(head_shape_fname=None) if dataset == "erm_HFH" else dict()
+    raw = mne.io.read_raw_bti(pdf_fname, **kwargs, verbose=False)
+
+    dest = tmp_path / dataset
+    copyfile_bti(raw=raw, dest=dest)
+
+    mne.io.read_raw_bti(dest / "c,rfDC", **kwargs, verbose=False)

mne_bids/tests/test_path.py

@@ -332,7 +332,7 @@ def test_parse_ext():
     assert fname == "sub-05_task-matchingpennies"
     assert ext == ".vhdr"
 
-    # Test for case where no extension: assume BTI format
+    # Test for case where no extension: assume BTi format
     f = "sub-01_task-rest"
     fname, ext = _parse_ext(f)
     assert fname == f

mne_bids/tests/test_write.py

@@ -1187,17 +1187,27 @@ def test_ctf(_bids_validate, tmp_path):
         get_anonymization_daysback(raw)
 
 
+@pytest.mark.parametrize("dataset", ("inbuilt_linux", "4Dsim", "erm_HFH"))
+@testing.requires_testing_data
 @pytest.mark.filterwarnings(warning_str["channel_unit_changed"])
-def test_bti(_bids_validate, tmp_path):
+def test_bti(_bids_validate, tmp_path, dataset):
     """Test functionality of the write_raw_bids conversion for BTi data."""
-    bti_path = op.join(base_path, "bti", "tests", "data")
-    raw_fname = op.join(bti_path, "test_pdf_linux")
-    config_fname = op.join(bti_path, "test_config_linux")
-    headshape_fname = op.join(bti_path, "test_hs_linux")
+    pytest.importorskip("mne", "1.5.0.dev")  # XXX: remove when mne<1.5 is dropped
+    if dataset == "inbuilt_linux":
+        bti_path = op.join(base_path, "bti", "tests", "data")
+        pdf_fname = op.join(bti_path, "test_pdf_linux")
+        kwargs = dict(
+            config_fname=op.join(bti_path, "test_config_linux"),
+            head_shape_fname=op.join(bti_path, "test_hs_linux"),
+        )
+    elif dataset == "4Dsim":
+        pdf_fname = data_path / "BTi" / dataset / "c,rfDC"
+        kwargs = dict()
+    else:
+        pdf_fname = data_path / "BTi" / dataset / "c,rfDC"
+        kwargs = dict(head_shape_fname=None)
 
-    raw = _read_raw_bti(
-        raw_fname, config_fname=config_fname, head_shape_fname=headshape_fname
-    )
+    raw = _read_raw_bti(pdf_fname, **kwargs)
 
     bids_path = _bids_path.copy().update(root=tmp_path, datatype="meg")
 
@@ -1216,18 +1226,22 @@ def test_bti(_bids_validate, tmp_path):
     assert op.exists(tmp_path / "participants.tsv")
     _bids_validate(tmp_path)
 
-    raw = read_raw_bids(bids_path=bids_path)
+    if dataset == "inbuilt_linux":
+        # Reading this is impossible, because the pdf file was renamed
+        # to some idiosyncratic name
+        with pytest.raises(RuntimeError, match="Cannot find BTi .*"):
+            raw = read_raw_bids(bids_path=bids_path)
 
-    with pytest.raises(TypeError, match="unexpected keyword argument 'foo'"):
-        read_raw_bids(bids_path=bids_path, extra_params=dict(foo="bar"))
+        # also test anonymize
+        raw = _read_raw_bti(pdf_fname, **kwargs)
+        with pytest.warns(RuntimeWarning, match="Converting to FIF for anonymization"):
+            output_path = _test_anonymize(tmp_path / "tmp", raw, bids_path)
+        _bids_validate(output_path)
 
-    # test anonymize
-    raw = _read_raw_bti(
-        raw_fname, config_fname=config_fname, head_shape_fname=headshape_fname
-    )
-    with pytest.warns(RuntimeWarning, match="Converting to FIF for anonymization"):
-        output_path = _test_anonymize(tmp_path / "tmp", raw, bids_path)
-    _bids_validate(output_path)
+    else:
+        # The other datasets can be read, as their pdf file names were
+        # not changed and still fit the "standard" naming patterns
+        raw = read_raw_bids(bids_path=bids_path)
 
 
 @pytest.mark.filterwarnings(

mne_bids/write.py

@@ -877,9 +877,9 @@ def _sidecar_json(
                 FIFF.FIFFV_POINT_LPA,
             ]:
                 digitized_landmark = True
-            elif dig_point["kind"] == FIFF.FIFFV_POINT_EXTRA and raw.filenames[
-                0
-            ].endswith(".fif"):
+            elif dig_point["kind"] == FIFF.FIFFV_POINT_EXTRA and str(
+                raw.filenames[0]
+            ).endswith(".fif"):
                 digitized_head_points = True
     software_filters = {
         "SpatialCompensation": {"GradientOrder": raw.compensation_grade}
@@ -1685,9 +1685,9 @@ def write_raw_bids(
         if ".ds" in op.dirname(raw.filenames[0]):
             raw_fname = op.dirname(raw.filenames[0])
         # point to file containing header info for multifile systems
-        raw_fname = raw_fname.replace(".eeg", ".vhdr")
-        raw_fname = raw_fname.replace(".fdt", ".set")
-        raw_fname = raw_fname.replace(".dat", ".lay")
+        raw_fname = str(raw_fname).replace(".eeg", ".vhdr")
+        raw_fname = str(raw_fname).replace(".fdt", ".set")
+        raw_fname = str(raw_fname).replace(".dat", ".lay")
         _, ext = _parse_ext(raw_fname)
 
         # force all EDF/BDF files with upper-case extension to be written as