[WIP] : simplify fpath

doc/whats_new.rst

@@ -51,6 +51,7 @@ API changes
 ^^^^^^^^^^^
 
 - Add ``format`` kwarg to :func:`write_raw_bids` that allows users to specify if they want to force conversion to ``BrainVision`` or ``FIF`` file format, by `Adam Li`_ (:gh:`672`)
+- :func:`mne_bids.BIDSPath.fpath` does not infer a full filepath anymore if the ``suffix``, or ``extension`` is missing. It returns the file path defined by the existing entities, by `Adam Li`_ and `Alexander Gramfort`_ (:gh:`721`)
 
 Requirements
 ^^^^^^^^^^^^

mne_bids/path.py

@@ -19,7 +19,7 @@
 from mne_bids.config import (
     ALLOWED_PATH_ENTITIES, ALLOWED_FILENAME_EXTENSIONS,
     ALLOWED_FILENAME_SUFFIX, ALLOWED_PATH_ENTITIES_SHORT,
-    ALLOWED_DATATYPES, SUFFIX_TO_DATATYPE, ALLOWED_DATATYPE_EXTENSIONS,
+    ALLOWED_DATATYPES, SUFFIX_TO_DATATYPE,
     ALLOWED_SPACES,
     reader, ENTITY_VALUE_TYPE)
 from mne_bids.utils import (_check_key_val, _check_empty_room_basename,
@@ -199,9 +199,10 @@ class BIDSPath(object):
         The "data type" of folder being created at the end of the folder
         hierarchy. E.g., ``'anat'``, ``'func'``, ``'eeg'``, ``'meg'``,
         ``'ieeg'``, etc.
-    root : str | pathlib.Path | None
+    root : str | pathlib.Path
         The root for the filename to be created. E.g., a path to the folder
         in which you wish to create a file with this name.
+        Defaults to ``'.'``, i.e. the current working directory.
     check : bool
         If True enforces the entities to be valid according to the
         current BIDS standard. Defaults to True.
@@ -232,7 +233,7 @@ class BIDSPath(object):
     Examples
     --------
     >>> bids_path = BIDSPath(subject='test', session='two', task='mytask',
-                                 suffix='ieeg', extension='.edf')
+                             suffix='ieeg', extension='.edf')
     >>> print(bids_path.basename)
     sub-test_ses-two_task-mytask_ieeg.edf
     >>> bids_path
@@ -278,11 +279,11 @@ class BIDSPath(object):
 
     def __init__(self, subject=None, session=None,
                  task=None, acquisition=None, run=None, processing=None,
-                 recording=None, space=None, split=None, root=None,
+                 recording=None, space=None, split=None, root='.',
                  suffix=None, extension=None, datatype=None, check=True):
         if all(ii is None for ii in [subject, session, task,
                                      acquisition, run, processing,
-                                     recording, space, root, suffix,
+                                     recording, space, suffix, root,
                                      extension]):
             raise ValueError("At least one parameter must be given.")
 
@@ -426,6 +427,8 @@ def fpath(self):
         # get the inner-most BIDS directory for this file path
         data_path = self.directory
 
+        _validate_type(self.root, types=['str', 'path-like'])
+
         # account for MEG data that are directory-based
         # else, all other file paths attempt to match
         if self.suffix == 'meg' and self.extension == '.ds':
@@ -434,57 +437,7 @@ def fpath(self):
             bids_fpath = op.join(data_path,
                                  op.splitext(self.basename)[0])
         else:
-            # if suffix and/or extension is missing, and root is
-            # not None, then BIDSPath will infer the dataset
-            # else, return the relative path with the basename
-            if (self.suffix is None or self.extension is None) and \
-                    self.root is not None:
-                # get matching BIDS paths inside the bids root
-                matching_paths = \
-                    _get_matching_bidspaths_from_filesystem(self)
-
-                # FIXME This will break
-                # FIXME e.g. with FIFF data split across multiple files.
-                # if extension is not specified and no unique file path
-                # return filepath of the actual dataset for MEG/EEG/iEEG data
-                if self.suffix is None or self.suffix in ALLOWED_DATATYPES:
-                    # now only use valid datatype extension
-                    valid_exts = sum(ALLOWED_DATATYPE_EXTENSIONS.values(), [])
-                    matching_paths = [p for p in matching_paths
-                                      if _parse_ext(p)[1] in valid_exts]
-
-                if (self.split is None and
-                        (not matching_paths or
-                         '_split-' in matching_paths[0])):
-                    # try finding FIF split files (only first one)
-                    this_self = self.copy().update(split='01')
-                    matching_paths = \
-                        _get_matching_bidspaths_from_filesystem(this_self)
-
-                # found no matching paths
-                if not matching_paths:
-                    msg = (f'Could not locate a data file of a supported '
-                           f'format. This is likely a problem with your '
-                           f'BIDS dataset. Please run the BIDS validator '
-                           f'on your data. (root={self.root}, '
-                           f'basename={self.basename}). '
-                           f'{matching_paths}')
-                    warn(msg)
-
-                    bids_fpath = op.join(data_path, self.basename)
-                # if paths still cannot be resolved, then there is an error
-                elif len(matching_paths) > 1:
-                    msg = ('Found more than one matching data file for the '
-                           'requested recording. Cannot proceed due to the '
-                           'ambiguity. This is likely a problem with your '
-                           'BIDS dataset. Please run the BIDS validator on '
-                           'your data.')
-                    raise RuntimeError(msg)
-                else:
-                    bids_fpath = matching_paths[0]
-
-            else:
-                bids_fpath = op.join(data_path, self.basename)
+            bids_fpath = op.join(data_path, self.basename)
 
         bids_fpath = Path(bids_fpath)
         return bids_fpath

mne_bids/tests/test_path.py

@@ -354,59 +354,30 @@ def test_find_matching_sidecar(return_bids_test_dir):
 
 
 def test_bids_path_inference(return_bids_test_dir):
-    """Test usage of BIDSPath object and fpath."""
+    """Test usage of BIDSPath object and fpath.
+
+    BIDSPath.fpath will not infer the file path in v0.7+
+    """
     bids_root = return_bids_test_dir
 
     # without providing all the entities, ambiguous when trying
     # to use fpath
     bids_path = BIDSPath(
-        subject=subject_id, session=session_id, acquisition=acq,
+        subject=subject_id, session=session_id,
         task=task, root=bids_root)
-    with pytest.raises(RuntimeError, match='Found more than one'):
-        bids_path.fpath
-
-    # can't locate a file, but the basename should work
-    bids_path = BIDSPath(
-        subject=subject_id, session=session_id, acquisition=acq,
-        task=task, run='10', root=bids_root)
-    with pytest.warns(RuntimeWarning, match='Could not locate'):
-        fpath = bids_path.fpath
-        assert str(fpath) == op.join(bids_root, f'sub-{subject_id}',
-                                     f'ses-{session_id}',
-                                     bids_path.basename)
-
-    # shouldn't error out when there is no uncertainty
-    channels_fname = BIDSPath(subject=subject_id, session=session_id,
-                              run=run, acquisition=acq, task=task,
-                              root=bids_root, suffix='channels')
-    channels_fname.fpath
-
-    # create an extra file under 'eeg'
-    extra_file = op.join(bids_root, f'sub-{subject_id}',
-                         f'ses-{session_id}', 'eeg',
-                         channels_fname.basename + '.tsv')
-    Path(extra_file).parent.mkdir(exist_ok=True, parents=True)
-    # Creates a new file and because of this new file, there is now
-    # ambiguity
-    with open(extra_file, 'w', encoding='utf-8'):
-        pass
-    with pytest.raises(RuntimeError, match='Found data of more than one'):
-        channels_fname.fpath
-
-    # if you set datatype, now there is no ambiguity
-    channels_fname.update(datatype='eeg')
-    assert str(channels_fname.fpath) == extra_file
-    # set state back to original
-    shutil.rmtree(Path(extra_file).parent)
+    expected_fpath = f'{bids_root}/sub-{subject_id}/ses-{session_id}/sub-{subject_id}_ses-{session_id}_task-{task}'  # noqa
+    assert bids_path.fpath.as_posix() == expected_fpath
 
 
 def test_bids_path(return_bids_test_dir):
     """Test usage of BIDSPath object."""
     bids_root = return_bids_test_dir
 
-    bids_path = BIDSPath(
+    bids_path_kwargs = dict(
         subject=subject_id, session=session_id, run=run, acquisition=acq,
-        task=task, root=bids_root, suffix='meg')
+        task=task, suffix='meg', extension='.fif'
+    )
+    bids_path = BIDSPath(root=bids_root, **bids_path_kwargs)
 
     expected_parent_dir = op.join(bids_root, f'sub-{subject_id}',
                                   f'ses-{session_id}', 'meg')
@@ -420,10 +391,10 @@ def test_bids_path(return_bids_test_dir):
     assert op.dirname(bids_path.fpath).startswith(bids_root)
 
     # when bids root is not passed in, passes relative path
-    bids_path2 = bids_path.copy().update(datatype='meg', root=None)
+    bids_path2 = BIDSPath(datatype='meg', **bids_path_kwargs)
     expected_relpath = op.join(
         f'sub-{subject_id}', f'ses-{session_id}', 'meg',
-        expected_basename + '_meg')
+        expected_basename + '_meg.fif')
     assert str(bids_path2.fpath) == expected_relpath
 
     # without bids_root and with suffix/extension
@@ -547,7 +518,7 @@ def test_bids_path(return_bids_test_dir):
                          task='03', suffix='ieeg',
                          extension='.edf')
     assert repr(bids_path) == ('BIDSPath(\n'
-                               'root: None\n'
+                               'root: .\n'
                                'datatype: ieeg\n'
                                'basename: sub-01_ses-02_task-03_ieeg.edf)')
 
@@ -560,10 +531,10 @@ def test_bids_path(return_bids_test_dir):
     assert bids_path.basename == 'sub-01_ses-02_task-03_split-01_ieeg.mat'
 
     # test home dir expansion
-    bids_path = BIDSPath(root='~/foo')
+    bids_path = BIDSPath(root='~/foo', subject='test')
     assert '~/foo' not in str(bids_path.root)
     # explicitly test update() method too
-    bids_path.update(root='~/foo')
+    bids_path.update(root='~/foo', subject='test')
     assert '~/foo' not in str(bids_path.root)
 
 
@@ -589,7 +560,7 @@ def test_make_filenames():
         BIDSPath(subject='one-two', suffix='ieeg', extension='.edf')
 
     with pytest.raises(ValueError, match='At least one'):
-        BIDSPath()
+        BIDSPath(root=None)
 
     # emptyroom check: invalid task
     with pytest.raises(ValueError, match='task must be'):