Repository: bids-standard/pybids
Pull request #281 (2018-11-07 20:41:13)

Default to using return_type='object' in get() calls
This PR changes the default `return_type` in `BIDSLayout.get()` calls from `'tuple'` to `'object'`. This is intended to help deprecate the use of the `_File` namedtuple in pybids (though it's still supported in grabbit, and hence remains available for use if a user manually requests `return_type='tuple'`). There probably isn't a good reason to have both namedtuples and `BIDSFile` objects.

To facilitate the shift, `BIDSFile` objects now allow entities to be accessed as attributes, mimicking the old namedtuple behavior. This means that the only potential backward incompatibility is that the `.filename` attribute of the namedtuple returns the full path to files, while the `.filename` attribute in `BIDSFile` only returns the basename of the file.


Patch:
diff --git a/bids/layout/layout.py b/bids/layout/layout.py
index 3b0e2cd1..15c772ae 100644
--- a/bids/layout/layout.py
+++ b/bids/layout/layout.py
@@ -32,6 +32,18 @@ class BIDSFile(File):
         super(BIDSFile, self).__init__(filename)
         self.layout = layout
 
+    def __getattr__(self, attr):
+        # Ensures backwards compatibility with old File_ namedtuple, which is
+        # deprecated as of 0.7.
+        if attr in self.entities:
+            warnings.warn("Accessing entities as attributes is deprecated as "
+                          "of 0.7. Please use the .entities dictionary instead"
+                          " (i.e., .entities['%s'] instead of .%s."
+                          % (attr, attr))
+            return self.entities[attr]
+        raise AttributeError("%s object has no attribute named %r" %
+                             (self.__class__.__name__, attr))
+
     @property
     def image(self):
         """ Return the associated image file (if it exists) as a NiBabel object.
@@ -277,7 +289,7 @@ class BIDSLayout(Layout):
         else:
             return None
 
-    def get(self, return_type='tuple', target=None, extensions=None,
+    def get(self, return_type='object', target=None, extensions=None,
             derivatives=True, regex_search=None, defined_fields=None,
             domains=None, **kwargs):
         """
@@ -285,13 +297,11 @@ class BIDSLayout(Layout):
 
         Args:
             return_type (str): Type of result to return. Valid values:
-                'tuple': returns a list of namedtuples containing file name as
-                    well as attribute/value pairs for all named entities.
-                'file': returns a list of matching filenames.
-                'dir': returns a list of directories.
-                'id': returns a list of unique IDs. Must be used together with
+                'object' (default): return a list of matching BIDSFile objects.
+                'file': return a list of matching filenames.
+                'dir': return a list of directories.
+                'id': return a list of unique IDs. Must be used together with
                     a valid target.
-                'obj': returns a list of matching File objects.
             target (str): Optional name of the target entity to get results for
                 (only used if return_type is 'dir' or 'id').
             extensions (str, list): One or more file extensions to filter on.
@@ -317,7 +327,7 @@ class BIDSLayout(Layout):
                 only files that match the first two subjects.
 
         Returns:
-            A named tuple (default) or a list (see return_type for details).
+            A list of BIDSFile (default) or other (see return_type for details) objects.
         """
 
         if derivatives == True:
@@ -454,36 +464,36 @@ class BIDSLayout(Layout):
         files = self.get(subject=sub, suffix=suffix,
                          extensions=['nii.gz', 'nii'])
         for file in files:
-            metadata = self.get_metadata(file.filename)
+            metadata = self.get_metadata(file.path)
             if metadata and "IntendedFor" in metadata.keys():
                 intended_for = listify(metadata["IntendedFor"])
                 if any([path.endswith(_suff) for _suff in intended_for]):
                     cur_fieldmap = {}
                     if file.suffix == "phasediff":
-                        cur_fieldmap = {"phasediff": file.filename,
-                                        "magnitude1": file.filename.replace(
+                        cur_fieldmap = {"phasediff": file.path,
+                                        "magnitude1": file.path.replace(
                                             "phasediff", "magnitude1"),
                                         "suffix": "phasediff"}
-                        magnitude2 = file.filename.replace(
+                        magnitude2 = file.path.replace(
                             "phasediff", "magnitude2")
                         if os.path.isfile(magnitude2):
                             cur_fieldmap['magnitude2'] = magnitude2
                     elif file.suffix == "phase1":
-                        cur_fieldmap["phase1"] = file.filename
+                        cur_fieldmap["phase1"] = file.path
                         cur_fieldmap["magnitude1"] = \
-                            file.filename.replace("phase1", "magnitude1")
+                            file.path.replace("phase1", "magnitude1")
                         cur_fieldmap["phase2"] = \
-                            file.filename.replace("phase1", "phase2")
+                            file.path.replace("phase1", "phase2")
                         cur_fieldmap["magnitude2"] = \
-                            file.filename.replace("phase1", "magnitude2")
+                            file.path.replace("phase1", "magnitude2")
                         cur_fieldmap["suffix"] = "phase"
                     elif file.suffix == "epi":
-                        cur_fieldmap["epi"] = file.filename
+                        cur_fieldmap["epi"] = file.path
                         cur_fieldmap["suffix"] = "epi"
                     elif file.suffix == "fieldmap":
-                        cur_fieldmap["fieldmap"] = file.filename
+                        cur_fieldmap["fieldmap"] = file.path
                         cur_fieldmap["magnitude"] = \
-                            file.filename.replace("fieldmap", "magnitude")
+                            file.path.replace("fieldmap", "magnitude")
                         cur_fieldmap["suffix"] = "fieldmap"
                     fieldmap_set.append(cur_fieldmap)
         return fieldmap_set
diff --git a/bids/reports/parsing.py b/bids/reports/parsing.py
index 835742af..b956982a 100644
--- a/bids/reports/parsing.py
+++ b/bids/reports/parsing.py
@@ -426,7 +426,7 @@ def parse_niftis(layout, niftis, subj, config, **kwargs):
     description_list = []
     skip_task = {}  # Only report each task once
     for nifti_struct in niftis:
-        nii_file = nifti_struct.filename
+        nii_file = nifti_struct.path
         metadata = layout.get_metadata(nii_file)
         if not metadata:
             LOGGER.warning('No json file found for %s', nii_file)
diff --git a/bids/reports/report.py b/bids/reports/report.py
index 4d2670a6..2dd099be 100644
--- a/bids/reports/report.py
+++ b/bids/reports/report.py
@@ -127,7 +127,7 @@ class BIDSReport(object):
                 description_list += parsing.parse_niftis(self.layout, niftis,
                                                          subject, self.config,
                                                          session=ses)
-                metadata = self.layout.get_metadata(niftis[0].filename)
+                metadata = self.layout.get_metadata(niftis[0].path)
             else:
                 raise Exception('No niftis for subject {0}'.format(subject))
 
diff --git a/bids/variables/io.py b/bids/variables/io.py
index 8b2e3b7f..08108eab 100644
--- a/bids/variables/io.py
+++ b/bids/variables/io.py
@@ -219,7 +219,7 @@ def _load_time_variables(layout, dataset=None, columns=None, scan_length=None,
                         if k in BASE_ENTITIES}
             confound_files = layout.get(suffix='confounds', **sub_ents)
             for cf in confound_files:
-                _data = pd.read_csv(cf.filename, sep='\t', na_values='n/a')
+                _data = pd.read_csv(cf.path, sep='\t', na_values='n/a')
                 if columns is not None:
                     conf_cols = list(set(_data.columns) & set(columns))
                     _data = _data.loc[:, conf_cols]


Test patch:
diff --git a/bids/layout/tests/test_layout.py b/bids/layout/tests/test_layout.py
index ebfc995b..18fb2d0f 100644
--- a/bids/layout/tests/test_layout.py
+++ b/bids/layout/tests/test_layout.py
@@ -137,10 +137,10 @@ def test_get_metadata_via_bidsfile(layout_7t_trt):
 
 def test_get_bvals_bvecs(layout_ds005):
     dwifile = layout_ds005.get(subject="01", datatype="dwi")[0]
-    result = layout_ds005.get_bval(dwifile.filename)
+    result = layout_ds005.get_bval(dwifile.path)
     assert result == abspath(join(layout_ds005.root, 'dwi.bval'))
 
-    result = layout_ds005.get_bvec(dwifile.filename)
+    result = layout_ds005.get_bvec(dwifile.path)
     assert result == abspath(join(layout_ds005.root, 'dwi.bvec'))