Repository: bids-standard/pybids
Pull request #593 (2020-04-07 20:15:53)

ENH: Add BIDSMetadata dictionary to report file with missing metadata
As discussed in poldracklab/fitlins#223, a common idiom is to retrieve metadata as a dictionary and then query it, resulting in opaque `KeyError`s when the sought term is absent. This PR makes a simple subclass of `dict` that retains the name of the file the metadata dictionary was produced by for inclusion in the error message.

The advantage of this approach is that downstream tools will get the reporting improvement for free without needing to change their method of fetching metadata.


Patch:
diff --git a/bids/layout/layout.py b/bids/layout/layout.py
index 6b255c66..c04cb845 100644
--- a/bids/layout/layout.py
+++ b/bids/layout/layout.py
@@ -21,6 +21,7 @@ from ..external import inflect
 from .writing import build_path, write_contents_to_file
 from .models import (Base, Config, BIDSFile, Entity, Tag)
 from .index import BIDSLayoutIndexer
+from .utils import BIDSMetadata
 from .. import config as cf
 
 try:
@@ -1146,6 +1147,7 @@ class BIDSLayout(object):
         precedence, per the inheritance rules in the BIDS specification.
 
         """
+        md = BIDSMetadata(str(path))
         for layout in self._get_layouts_in_scope(scope):
 
             query = (layout.session.query(Tag)
@@ -1157,9 +1159,10 @@ class BIDSLayout(object):
 
             results = query.all()
             if results:
-                return {t.entity_name: t.value for t in results}
+                md.update({t.entity_name: t.value for t in results})
+                return md
 
-        return {}
+        return md
 
     def get_dataset_description(self, scope='self', all_=False):
         """Return contents of dataset_description.json.
diff --git a/bids/layout/models.py b/bids/layout/models.py
index 054476b9..9b1e0893 100644
--- a/bids/layout/models.py
+++ b/bids/layout/models.py
@@ -14,6 +14,7 @@ from itertools import chain
 
 from .writing import build_path, write_contents_to_file
 from ..config import get_option
+from .utils import BIDSMetadata
 
 Base = declarative_base()
 
@@ -205,7 +206,9 @@ class BIDSFile(Base):
 
     def get_metadata(self):
         """Return all metadata associated with the current file. """
-        return self.get_entities(metadata=True)
+        md = BIDSMetadata(self.path)
+        md.update(self.get_entities(metadata=True))
+        return md
 
     def get_entities(self, metadata=False, values='tags'):
         """Return entity information for the current file.
diff --git a/bids/layout/utils.py b/bids/layout/utils.py
new file mode 100644
index 00000000..847b7408
--- /dev/null
+++ b/bids/layout/utils.py
@@ -0,0 +1,12 @@
+class BIDSMetadata(dict):
+    """ Metadata dictionary that reports the associated file on lookup failures. """
+    def __init__(self, source_file):
+        self._source_file = source_file
+        super().__init__()
+
+    def __getitem__(self, key):
+        try:
+            return super().__getitem__(key)
+        except KeyError as e:
+            raise KeyError(
+                "Metadata term {!r} unavailable for file {}.".format(key, self._source_file))
diff --git a/setup.cfg b/setup.cfg
index 676ee0dc..86d053a9 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -40,7 +40,7 @@ include_package_data = True
 [options.extras_require]
 analysis =
 doc =
-    sphinx >=2.2
+    sphinx >=2.2,<3
     numpydoc
     m2r
     sphinx_rtd_theme


Test patch:
diff --git a/bids/layout/tests/test_layout.py b/bids/layout/tests/test_layout.py
index 9275e292..52647802 100644
--- a/bids/layout/tests/test_layout.py
+++ b/bids/layout/tests/test_layout.py
@@ -14,6 +14,7 @@ from bids.layout import (BIDSLayout, parse_file_entities, add_config_paths,
                          Query)
 from bids.layout.index import BIDSLayoutIndexer
 from bids.layout.models import Entity, Config
+from bids.layout.utils import BIDSMetadata
 from bids.tests import get_test_data_path
 from bids.utils import natural_sort
 
@@ -175,6 +176,22 @@ def test_get_metadata_via_bidsfile(layout_7t_trt):
     assert 'subject' not in result
 
 
+def test_get_metadata_error(layout_7t_trt):
+    ''' Same as test_get_metadata5, but called through BIDSFile. '''
+    target = 'sub-01/ses-1/func/sub-01_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz'
+    target = target.split('/')
+    path = join(layout_7t_trt.root, *target)
+    result = layout_7t_trt.files[path].get_metadata()
+    with pytest.raises(KeyError) as err:
+        result['Missing']
+    assert "Metadata term 'Missing' unavailable for file {}".format(path) in str(err)
+
+    result = layout_7t_trt.get_metadata(path)
+    with pytest.raises(KeyError) as err:
+        result['Missing']
+    assert "Metadata term 'Missing' unavailable for file {}".format(path) in str(err)
+
+
 def test_get_with_bad_target(layout_7t_trt):
     with pytest.raises(ValueError) as exc:
         layout_7t_trt.get(target='unicorn')
diff --git a/bids/layout/tests/test_utils.py b/bids/layout/tests/test_utils.py
new file mode 100644
index 00000000..603ecced
--- /dev/null
+++ b/bids/layout/tests/test_utils.py
@@ -0,0 +1,11 @@
+import pytest
+from ..utils import BIDSMetadata
+
+
+def test_bidsmetadata_class():
+    md = BIDSMetadata("fakefile")
+    with pytest.raises(KeyError) as err:
+        md["Missing"]
+    assert "Metadata term 'Missing' unavailable for file fakefile." in str(err)
+    md["Missing"] = 1
+    assert md["Missing"] == 1