MRG: Deprecate silently adding a leading period to an extension in BIDSPath

doc/whats_new.rst

@@ -53,7 +53,7 @@ Detailed list of changes
 
 - :class:`~mne_bids.BIDSPath` now supports the BIDS "description" entity ``desc``, used in derivative data, by `Richard Höchenberger`_ (:gh:`1049`)
 
-- Added support for ``GSR`` (galvanic skin response / electrodermal activity, EDA) and ``TEMP`` (temperature) channel types, by `Richard Höchenberger`_ (:gh:`xxx`)
+- Added support for ``GSR`` (galvanic skin response / electrodermal activity, EDA) and ``TEMP`` (temperature) channel types, by `Richard Höchenberger`_ (:gh:`1059`)
 
 🧐 API and behavior changes
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -72,6 +72,8 @@ Detailed list of changes
 
 - Passing only one of ``events`` and ``event_id`` to :func:`~mne_bids.write_raw_bids` now raises a ``ValueError`` instead of a ``RuntimeError``, by `Richard Höchenberger`_ (:gh:`1054`)
 
+- Until now, :class:`mne_bids.BIDSPath` prepends extensions with a period "." automatically. We intend to remove this undocumented side-effect and now emit a ``FutureWarning`` if an ``extension`` that does not start with a ``.`` is provided. Starting with MNE-BIDS 0.12, an exception will be raised in this case, by `Richard Höchenberger`_ (:gh:`1061`)
+
 🛠 Requirements
 ^^^^^^^^^^^^^^^
 

mne_bids/path.py

@@ -726,9 +726,21 @@ def update(self, *, check=None, **kwargs):
 
         # ensure extension starts with a '.'
         extension = kwargs.get('extension')
-        if extension is not None:
-            if not extension.startswith('.'):
-                kwargs['extension'] = f'.{extension}'
+        if extension is not None and not extension.startswith('.'):
+            warn(
+                f'extension should start with a period ".", but got: '
+                f'"{extension}". Prepending "." to form: ".{extension}". '
+                f'This will raise an exception starting with MNE-BIDS 0.12.',
+                category=FutureWarning
+            )
+            kwargs['extension'] = f'.{extension}'
+            # Uncomment in 0.12, and remove above code:
+            #
+            # raise ValueError(
+            #     f'Extension must start wie a period ".", but got: '
+            #     f'{extension}'
+            # )
+        del extension
 
         # error check entities
         old_kwargs = dict()
@@ -1298,10 +1310,13 @@ def get_bids_path_from_fname(fname, check=True, verbose=None):
         suffix, extension = _get_bids_suffix_and_ext(suffix)
     else:
         suffix = None
-        extension = Path(fname).suffix
+        extension = Path(fname).suffix  # already starts with a period
         if extension == '':
             extension = None
 
+    if extension is not None:
+        assert extension.startswith('.')  # better safe than sorry
+
     datatype = _infer_datatype_from_path(fpath)
 
     # find root and datatype if it exists
@@ -1517,6 +1532,7 @@ def _get_bids_suffix_and_ext(str_suffix):
         split_str = str_suffix.split('.')
         suffix = split_str[0]
         ext = '.'.join(split_str[1:])
+        ext = f'.{ext}'  # prepend period
     return suffix, ext
 
 

mne_bids/stats.py

@@ -40,7 +40,7 @@ def count_events(root_or_path, datatype='auto'):
     else:
         bids_path = root_or_path.copy()
 
-    bids_path.update(suffix='events', extension='tsv')
+    bids_path.update(suffix='events', extension='.tsv')
 
     datatypes = get_datatypes(bids_path.root)
     this_datatypes = list(set(datatypes).intersection(EPHY_ALLOWED_DATATYPES))

mne_bids/tests/test_path.py

@@ -510,7 +510,7 @@ def test_bids_path(return_bids_test_dir):
 
     # without bids_root and with suffix/extension
     # basename and fpath should be the same
-    bids_path.update(suffix='ieeg', extension='vhdr')
+    bids_path.update(suffix='ieeg', extension='.vhdr')
     expected_basename2 = expected_basename + '_ieeg.vhdr'
     assert (bids_path.basename == expected_basename2)
     bids_path.update(extension='.vhdr')
@@ -1165,3 +1165,9 @@ def test_setting_entities():
 
         setattr(bids_path, entity_name, None)
         assert getattr(bids_path, entity_name) is None
+
+
+def test_deprecation():
+    """Test deprecated behavior."""
+    with pytest.warns(FutureWarning, match='This will raise an exception'):
+        BIDSPath(extension='vhdr')  # no leading period

mne_bids/tests/test_write.py

@@ -535,6 +535,8 @@ def test_fif(_bids_validate, tmp_path):
     for sidecar in ['channels.tsv', 'eeg.eeg', 'eeg.json', 'eeg.vhdr',
                     'eeg.vmrk', 'events.tsv']:
         suffix, extension = sidecar.split('.')
+        extension = f'.{extension}'
+
         sidecar_basename.update(suffix=suffix, extension=extension)
         assert op.isfile(op.join(bids_dir, sidecar_basename.basename))