Repository: IAMconsortium/pyam (python)

Issue:
`filter()` assumes non-starred `dict` as `keep` kwarg
# Observed (user) error

Just ran into a stupid error where I used `df.filter(dict(foo='bar')` instead of `df.filter(**dict(foo='bar')`, so rather than applying the expected `foo='bar'` rule, the function interprets the dict as the `keep` kwarg.

# Suggested solution

Add a check that `keep` is a boolean, raise error otherwise.

Pull request:
add check that `keep` in `filter()` must be boolean
# Please confirm that this PR has done the following:

- [x] Tests Added
- [x] Documentation Added
- [x] Description in RELEASE_NOTES.md Added

# Description of PR

A non-starred `dict` in `df.filter()` is mis-interpreted as the `keep` kwarg, returning a copy of the full (rather than downselected) `IamDataFrame`. This PR adds a check the `keep` must be boolean.

closes #253


Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 3cf842e..bbd0b84 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,6 +1,7 @@
 
 # Next Release
 
+- [#261](https://github.com/IAMconsortium/pyam/pull/261) Add a check that `keep` in `filter()` is a boolean
 - [#243](https://github.com/IAMconsortium/pyam/pull/243) Update `pyam.iiasa.Connection` to support all public and private database connections. DEPRECATED: the argument 'iamc15' has been deprecated in favor of names as queryable directly from the REST API.
 - [#241](https://github.com/IAMconsortium/pyam/pull/241) Add `set_meta_from_data` feature
 - [#236](https://github.com/IAMconsortium/pyam/pull/236) Add `swap_time_for_year` method and confirm datetime column is compatible with pyam features
diff --git a/pyam/core.py b/pyam/core.py
index e945530..c563188 100644
--- a/pyam/core.py
+++ b/pyam/core.py
@@ -974,6 +974,10 @@ class IamDataFrame(object):
                ('month', 'hour', 'time')
              - 'regexp=True' disables pseudo-regexp syntax in `pattern_match()`
         """
+        if not isinstance(keep, bool):
+            msg = '`filter(keep={}, ...)` is not valid, must be boolean'
+            raise ValueError(msg.format(keep))
+
         _keep = self._apply_filters(**kwargs)
         _keep = _keep if keep else ~_keep
         ret = copy.deepcopy(self) if not inplace else self


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 3bc0596..e4df621 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -201,8 +201,15 @@ def test_variable_depth_raises(test_df):
     pytest.raises(ValueError, test_df.filter, level='1/')
 
 
-def test_filter_error(test_df):
-    pytest.raises(ValueError, test_df.filter, foo='foo')
+def test_filter_error_illegal_column(test_df):
+    # filtering by column `foo` is not valid
+    pytest.raises(ValueError, test_df.filter, foo='test')
+
+
+def test_filter_error_keep(test_df):
+    # string or non-starred dict was mis-interpreted as `keep` kwarg, see #253
+    pytest.raises(ValueError, test_df.filter, model='foo', keep=1)
+    pytest.raises(ValueError, test_df.filter, dict(model='foo'))
 
 
 def test_filter_year(test_df):