Repository: bids-standard/pybids

Issue:
get silently ignores unknown keyword arguments (in my case `ses`)
I am "inside" one of the tests, and decided to get a sample file.  To my surprise - I got multiple:
```
431  	def test_derivative_getters():
432  	    synth_path = join(get_test_data_path(), 'synthetic')
433  	    bare_layout = BIDSLayout(synth_path, derivatives=False)
434  	    full_layout = BIDSLayout(synth_path, derivatives=True)
435  	    import pdb; pdb.set_trace()
436  ->	    assert bare_layout.get_spaces() == []

*(Pdb) from pprint import pprint

(Pdb) pprint(bare_layout.get(subject='05', ses='02', suffix='T1w'))
[<BIDSImageFile filename='/home/yoh/proj/bids/pybids/bids/tests/data/synthetic/sub-05/ses-01/anat/sub-05_ses-01_T1w.nii'>,
 <BIDSImageFile filename='/home/yoh/proj/bids/pybids/bids/tests/data/synthetic/sub-05/ses-01/anat/sub-05_ses-01_T1w.nii.gz'>,
 <BIDSImageFile filename='/home/yoh/proj/bids/pybids/bids/tests/data/synthetic/sub-05/ses-02/anat/sub-05_ses-02_T1w.nii'>,
 <BIDSImageFile filename='/home/yoh/proj/bids/pybids/bids/tests/data/synthetic/sub-05/ses-02/anat/sub-05_ses-02_T1w.nii.gz'>]
(Pdb) bids.__version__
'0.10.2+12.g8b5ca0e.dirty'
```
it does select correctly when I correctly specify `session` instead of `ses`.  I would expect pybids to at least complain that whatever I have specified `ses` was not understood.  Typos, API changes, etc could silently lead to incorrect results, thus imho it warrants AT LEAST a warning (I feel now that we had some discussion like that before, sorry if I am repeating myself ;))

Discussion:

Patch:
diff --git a/bids/layout/layout.py b/bids/layout/layout.py
index e1acd6bd..5adba334 100644
--- a/bids/layout/layout.py
+++ b/bids/layout/layout.py
@@ -11,6 +11,7 @@ import warnings
 import sqlite3
 import enum
 from pathlib import Path
+import difflib
 
 import sqlalchemy as sa
 from sqlalchemy.orm import joinedload
@@ -848,7 +849,7 @@ class BIDSLayout(object):
         return data.reset_index()
 
     def get(self, return_type='object', target=None, scope='all',
-            regex_search=False, absolute_paths=None, drop_invalid_filters=True,
+            regex_search=False, absolute_paths=None, invalid_filters='error',
             **filters):
         """Retrieve files and/or metadata from the current Layout.
 
@@ -882,6 +883,14 @@ class BIDSLayout(object):
             to report either absolute or relative (to the top of the
             dataset) paths. If None, will fall back on the value specified
             at BIDSLayout initialization.
+        invalid_filters (str): Controls behavior when named filters are
+            encountered that don't exist in the database (e.g., in the case of
+            a typo like subbject='0.1'). Valid values:
+                'error' (default): Raise an explicit error.
+                'drop': Silently drop invalid filters (equivalent to not having
+                    passed them as arguments in the first place).
+                'allow': Include the invalid filters in the query, resulting
+                    in no results being returned.
         filters : dict
             Any optional key/values to filter the entities on.
             Keys are entity names, values are regexes to filter on. For
@@ -909,15 +918,25 @@ class BIDSLayout(object):
             filters['extension'] = [x.lstrip('.') if isinstance(x, str) else x
                                     for x in exts]
 
-        if drop_invalid_filters:
-            invalid_filters = set(filters.keys()) - set(entities.keys())
-            if invalid_filters:
-                for inv_filt in invalid_filters:
-                    filters.pop(inv_filt)
+        if invalid_filters != 'allow':
+            bad_filters = set(filters.keys()) - set(entities.keys())
+            if bad_filters:
+                if invalid_filters == 'drop':
+                    for bad_filt in bad_filters:
+                        filters.pop(bad_filt)
+                elif invalid_filters == 'error':
+                    first_bad = list(bad_filters)[0]
+                    msg = "'{}' is not a recognized entity. ".format(first_bad)
+                    ents = list(entities.keys())
+                    suggestions = difflib.get_close_matches(first_bad, ents)
+                    if suggestions:
+                        msg += "Did you mean {}? ".format(suggestions)
+                    raise ValueError(msg + "If you're sure you want to impose "
+                                     "this constraint, set "
+                                     "invalid_filters='allow'.")
 
         # Provide some suggestions if target is specified and invalid.
         if target is not None and target not in entities:
-            import difflib
             potential = list(entities.keys())
             suggestions = difflib.get_close_matches(target, potential)
             if suggestions:


Test patch:
diff --git a/bids/layout/tests/test_layout.py b/bids/layout/tests/test_layout.py
index 229634d9..e2a39375 100644
--- a/bids/layout/tests/test_layout.py
+++ b/bids/layout/tests/test_layout.py
@@ -40,7 +40,8 @@ def test_index_metadata(index_metadata, query, result):
     if not index_metadata and query is not None:
         indexer = BIDSLayoutIndexer(layout)
         indexer.index_metadata(**query)
-    sample_file = layout.get(task='rest', extension='nii.gz', acq='fullbrain')[0]
+    sample_file = layout.get(task='rest', extension='nii.gz',
+                             acquisition='fullbrain')[0]
     metadata = sample_file.get_metadata()
     assert metadata.get('RepetitionTime') == result
 
@@ -267,25 +268,25 @@ def test_get_return_type_dir(layout_7t_trt, layout_7t_trt_relpath):
 
 @pytest.mark.parametrize("acq", [None, Query.NONE])
 def test_get_val_none(layout_7t_trt, acq):
-    t1w_files = layout_7t_trt.get(subject='01', ses='1', suffix='T1w')
+    t1w_files = layout_7t_trt.get(subject='01', session='1', suffix='T1w')
     assert len(t1w_files) == 1
     assert 'acq' not in t1w_files[0].path
     t1w_files = layout_7t_trt.get(
-        subject='01', ses='1', suffix='T1w', acquisition=acq)
+        subject='01', session='1', suffix='T1w', acquisition=acq)
     assert len(t1w_files) == 1
     bold_files = layout_7t_trt.get(
-        subject='01', ses='1', suffix='bold', acquisition=acq)
+        subject='01', session='1', suffix='bold', acquisition=acq)
     assert len(bold_files) == 0
 
 
 def test_get_val_enum_any(layout_7t_trt):
     t1w_files = layout_7t_trt.get(
-        subject='01', ses='1', suffix='T1w', acquisition=Query.ANY,
+        subject='01', session='1', suffix='T1w', acquisition=Query.ANY,
         extension=Query.ANY)
     assert not t1w_files
-    bold_files = layout_7t_trt.get(subject='01', ses='1', run=1, suffix='bold',
-                                   acquisition=Query.ANY)
-    assert len(bold_files) == 3
+    bold_files = layout_7t_trt.get(subject='01', session='1', run=1,
+                                  suffix='bold', acquisition=Query.ANY)
+    assert len(bold_files) == 2
 
 
 def test_get_return_sorted(layout_7t_trt):
@@ -669,6 +670,26 @@ def test_get_with_regex_search_bad_dtype(layout_7t_trt):
     # Two runs (1 per session) for each of subjects '10' and '01'
     assert len(results) == 4
 
+
+def test_get_with_invalid_filters(layout_ds005):
+    l = layout_ds005
+    # Raise error with suggestions
+    with pytest.raises(ValueError, match='session'):
+        l.get(subject='12', ses=True, invalid_filters='error')
+    with pytest.raises(ValueError, match='session'):
+        l.get(subject='12', ses=True)
+    # Silently drop amazing
+    res_without = l.get(subject='12', suffix='bold')
+    res_drop = l.get(subject='12', suffix='bold', amazing='!!!',
+                     invalid_filters='drop')
+    assert res_without == res_drop
+    assert len(res_drop) == 3
+    # Retain amazing, producing empty set
+    allow_res = l.get(subject='12', amazing=True, invalid_filters='allow')
+    assert allow_res == []
+
+
+
 def test_load_layout(layout_synthetic_nodb, db_dir):
     db_path = str(db_dir / 'tmp_db')
     layout_synthetic_nodb.save(db_path)