Repository: IAMconsortium/pyam
Pull request #614 (2022-01-30 22:43:45)

Clean-up of `filter` implementation
# Description of PR

This PR migrates parts of the `filter`-implementation and their tests to own modules.


codecov[bot]: # [Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) Report
> Merging [#614](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) (f065c35) into [feature/year-and-datetime](https://codecov.io/gh/IAMconsortium/pyam/commit/49c9f7941b0409f02a5fb53a529b633d3d3314f9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) (49c9f79) will **increase** coverage by `0.0%`.
> The diff coverage is `99.0%`.

[![Impacted file tree graph](https://codecov.io/gh/IAMconsortium/pyam/pull/614/graphs/tree.svg?width=650&height=150&src=pr&token=ThxWlNkonJ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium)](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium)

```diff
@@                    Coverage Diff                    @@
##           feature/year-and-datetime    #614   +/-   ##
=========================================================
  Coverage                       94.3%   94.3%           
=========================================================
  Files                             55      56    +1     
  Lines                           5501    5508    +7     
=========================================================
+ Hits                            5190    5199    +9     
+ Misses                           311     309    -2     
```


| [Impacted Files](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium) | Coverage Δ | |
|---|---|---|
| [pyam/utils.py](https://codecov.io/gh/IAMconsortium/pyam/pull/614/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-cHlhbS91dGlscy5weQ==) | `90.8% <ø> (-0.2%)` | :arrow_down: |
| [tests/test\_core.py](https://codecov.io/gh/IAMconsortium/pyam/pull/614/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-dGVzdHMvdGVzdF9jb3JlLnB5) | `100.0% <ø> (ø)` | |
| [pyam/filter.py](https://codecov.io/gh/IAMconsortium/pyam/pull/614/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-cHlhbS9maWx0ZXIucHk=) | `97.2% <96.9%> (-2.8%)` | :arrow_down: |
| [pyam/core.py](https://codecov.io/gh/IAMconsortium/pyam/pull/614/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-cHlhbS9jb3JlLnB5) | `94.6% <100.0%> (-0.2%)` | :arrow_down: |
| [tests/test\_filter.py](https://codecov.io/gh/IAMconsortium/pyam/pull/614/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium#diff-dGVzdHMvdGVzdF9maWx0ZXIucHk=) | `100.0% <100.0%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium). Last update [49c9f79...f065c35](https://codecov.io/gh/IAMconsortium/pyam/pull/614?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=IAMconsortium).


Patch:
diff --git a/pyam/core.py b/pyam/core.py
index 6cb82cb..7eb4977 100755
--- a/pyam/core.py
+++ b/pyam/core.py
@@ -11,7 +11,7 @@ import pandas as pd
 from pathlib import Path
 from tempfile import TemporaryDirectory
 
-from pyam.filter import filter_by_time_domain
+from pyam.filter import filter_by_time_domain, filter_by_year, filter_by_dt_arg
 
 try:
     from datapackage import Package
@@ -38,10 +38,6 @@ from pyam.utils import (
     merge_meta,
     find_depth,
     pattern_match,
-    years_match,
-    day_match,
-    datetime_match,
-    time_match,
     isstr,
     islistable,
     print_list,
@@ -51,7 +47,9 @@ from pyam.utils import (
     IAMC_IDX,
     SORT_IDX,
     ILLEGAL_COLS,
-    FILTER_DATETIME_ATTRS,
+)
+from pyam.filter import (
+    datetime_match,
 )
 from pyam.read_ixmp import read_ix
 from pyam.plotting import PlotAccessor
@@ -75,7 +73,7 @@ from pyam.index import (
 )
 from pyam.time import swap_time_for_year, swap_year_for_time
 from pyam._debiasing import _compute_bias
-from pyam.logging import deprecation_warning, raise_data_error
+from pyam.logging import raise_data_error
 
 logger = logging.getLogger(__name__)
 
@@ -1785,45 +1783,14 @@ class IamDataFrame(object):
 
             elif col == "year":
                 levels, codes = get_index_levels_codes(self._data, self.time_col)
-                if self.time_col == "time":
-                    levels = [
-                        i.year if isinstance(i, pd.Timestamp) else i for i in levels
-                    ]
-                matches = years_match(levels, values)
-                keep_col = get_keep_col(codes, matches)
+                keep_col = filter_by_year(self.time_col, values, levels, codes)
 
-            elif col in ["month", "hour"]:
+            elif col in ["month", "hour", "day"]:
                 if self.time_col != "time":
                     logger.error(f"Filter by `{col}` not supported with yearly data.")
                     return np.zeros(len(self), dtype=bool)
 
-                def time_col(x, col):
-                    return getattr(x, col) if isinstance(x, pd.Timestamp) else None
-
-                data = self.get_data_column("time").apply(lambda x: time_col(x, col))
-                if col in FILTER_DATETIME_ATTRS:
-                    keep_col = time_match(data, values, *FILTER_DATETIME_ATTRS[col])
-                else:
-                    keep_col = np.isin(data, values)
-
-            elif col == "day":
-                if self.time_col != "time":
-                    logger.error(f"Filter by `{col}` not supported with yearly data.")
-                    return np.zeros(len(self), dtype=bool)
-
-                if isinstance(values, str):
-                    wday = True
-                elif isinstance(values, list) and isinstance(values[0], str):
-                    wday = True
-                else:
-                    wday = False
-
-                if wday:
-                    days = self.get_data_column("time").apply(lambda x: x.weekday())
-                else:  # ints or list of ints
-                    days = self.get_data_column("time").apply(lambda x: x.day)
-
-                keep_col = day_match(days, values)
+                keep_col = filter_by_dt_arg(col, values, self.get_data_column("time"))
 
             elif col == "time":
                 if self.time_col != "time":
@@ -1843,7 +1810,6 @@ class IamDataFrame(object):
                     has_nan=True,
                     return_codes=True,
                 )
-
                 keep_col = get_keep_col(codes, matches)
 
             else:
diff --git a/pyam/filter.py b/pyam/filter.py
index c5e1fa6..60a6e0c 100644
--- a/pyam/filter.py
+++ b/pyam/filter.py
@@ -1,4 +1,16 @@
+import datetime
+import time
 import numpy as np
+import pandas as pd
+
+from pyam.utils import to_list
+from pyam.index import get_keep_col
+
+
+FILTER_DATETIME_ATTRS = {
+    "month": (["%b", "%B"], "tm_mon", "months"),
+    "day": (["%a", "%A"], "tm_wday", "days"),
+}
 
 
 def filter_by_time_domain(values, levels, codes):
@@ -9,6 +21,106 @@ def filter_by_time_domain(values, levels, codes):
     elif values == "datetime":
         matches = [i for (i, label) in enumerate(levels) if not isinstance(label, int)]
     else:
-        raise ValueError(f"Filter by `datetime='{values}'` not supported!")
+        raise ValueError(f"Filter by `time_domain='{values}'` not supported!")
 
     return np.isin(codes, matches)
+
+
+def filter_by_year(time_col, values, levels, codes):
+    """Internal implementation to filter by time domain"""
+
+    if time_col == "time":
+        levels = [i.year if isinstance(i, pd.Timestamp) else i for i in levels]
+    return get_keep_col(codes, years_match(levels, values))
+
+
+def filter_by_dt_arg(col, values, data):
+    """Internal implementation to filter by datetime arguments"""
+
+    def time_col(x, col):
+        return getattr(x, col) if isinstance(x, pd.Timestamp) else None
+
+    if col == "day":
+        if isinstance(values, str):
+            wday = True
+        elif isinstance(values, list) and isinstance(values[0], str):
+            wday = True
+        else:
+            wday = False
+
+        if wday:
+            data = data.apply(lambda x: x.weekday())
+        else:  # ints or list of ints
+            data = data.apply(lambda x: x.day)
+
+    else:
+        data = data.apply(lambda x: time_col(x, col))
+
+    if col in FILTER_DATETIME_ATTRS:
+        return time_match(data, values, *FILTER_DATETIME_ATTRS[col])
+    else:
+        return np.isin(data, values)
+
+
+def years_match(levels, years):
+    """Return rows where data matches year"""
+    years = to_list(years)
+    if not all([pd.api.types.is_integer(y) for y in years]):
+        raise TypeError("Filter by `year` requires integers!")
+
+    return np.isin(levels, years)
+
+
+def time_match(data, times, conv_codes, strptime_attr, name):
+    """Return rows where data matches a timestamp"""
+
+    def conv_strs(strs_to_convert, conv_codes, name):
+        for conv_code in conv_codes:
+            try:
+                res = [
+                    getattr(time.strptime(t, conv_code), strptime_attr)
+                    for t in strs_to_convert
+                ]
+                break
+            except ValueError:
+                continue
+
+        try:
+            return res
+        except NameError:
+            raise ValueError(f"Could not convert {name} to integer: {times}")
+
+    times = [times] if isinstance(times, (int, str)) else times
+    if isinstance(times[0], str):
+        to_delete = []
+        to_append = []
+        for i, timeset in enumerate(times):
+            if "-" in timeset:
+                ints = conv_strs(timeset.split("-"), conv_codes, name)
+                if ints[0] > ints[1]:
+                    error_msg = (
+                        "string ranges must lead to increasing integer ranges,"
+                        " {} becomes {}".format(timeset, ints)
+                    )
+                    raise ValueError(error_msg)
+
+                # + 1 to include last month
+                to_append += [j for j in range(ints[0], ints[1] + 1)]
+                to_delete.append(i)
+
+        for i in to_delete:
+            del times[i]
+
+        times = conv_strs(times, conv_codes, name)
+        times += to_append
+
+    return np.isin(data, times)
+
+
+def datetime_match(data, dts):
+    """Matching of datetimes in time columns for data filtering"""
+    dts = to_list(dts)
+    if any([not (isinstance(i, (datetime.datetime, np.datetime64))) for i in dts]):
+        error_msg = "`time` can only be filtered by datetimes and datetime64s"
+        raise TypeError(error_msg)
+    return data.isin(dts).values
diff --git a/pyam/utils.py b/pyam/utils.py
index f54f9a6..92843ca 100644
--- a/pyam/utils.py
+++ b/pyam/utils.py
@@ -4,9 +4,7 @@ import logging
 import string
 import six
 import re
-import datetime
 import dateutil
-import time
 
 from pyam.index import get_index_levels, replace_index_labels
 from pyam.logging import raise_data_error
@@ -522,85 +520,6 @@ def _escape_regexp(s):
     )
 
 
-def years_match(levels, years):
-    """Return rows where data matches year"""
-    years = to_list(years)
-    if not all([pd.api.types.is_integer(y) for y in years]):
-        raise TypeError("Filter by `year` requires integers!")
-    return np.isin(levels, years)
-
-
-FILTER_DATETIME_ATTRS = {
-    "month": (["%b", "%B"], "tm_mon", "months"),
-}
-
-
-def day_match(data, days):
-    """Return rows where data matches days"""
-    return time_match(data, days, ["%a", "%A"], "tm_wday", "days")
-
-
-def hour_match(data, hours):
-    """Return rows where data matches hours"""
-    hours = [hours] if isinstance(hours, int) else hours
-    return np.isin(data, hours)
-
-
-def time_match(data, times, conv_codes, strptime_attr, name):
-    """Return rows where data matches a timestamp"""
-
-    def conv_strs(strs_to_convert, conv_codes, name):
-        for conv_code in conv_codes:
-            try:
-                res = [
-                    getattr(time.strptime(t, conv_code), strptime_attr)
-                    for t in strs_to_convert
-                ]
-                break
-            except ValueError:
-                continue
-
-        try:
-            return res
-        except NameError:
-            raise ValueError(f"Could not convert {name} to integer: {times}")
-
-    times = [times] if isinstance(times, (int, str)) else times
-    if isinstance(times[0], str):
-        to_delete = []
-        to_append = []
-        for i, timeset in enumerate(times):
-            if "-" in timeset:
-                ints = conv_strs(timeset.split("-"), conv_codes, name)
-                if ints[0] > ints[1]:
-                    error_msg = (
-                        "string ranges must lead to increasing integer ranges,"
-                        " {} becomes {}".format(timeset, ints)
-                    )
-                    raise ValueError(error_msg)
-
-                # + 1 to include last month
-                to_append += [j for j in range(ints[0], ints[1] + 1)]
-                to_delete.append(i)
-
-        for i in to_delete:
-            del times[i]
-
-        times = conv_strs(times, conv_codes, name)
-        times += to_append
-
-    return np.isin(data, times)
-
-
-def datetime_match(data, dts):
-    """Matching of datetimes in time columns for data filtering"""
-    dts = dts if islistable(dts) else [dts]
-    if any([not (isinstance(i, (datetime.datetime, np.datetime64))) for i in dts]):
-        error_msg = "`time` can only be filtered by datetimes and datetime64s"
-        raise TypeError(error_msg)
-    return data.isin(dts).values
-
-
 def print_list(x, n):
     """Return a printable string of a list shortened to n characters"""
     # if list is empty, only write count


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 4dcd4d9..d02f37f 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -412,246 +412,6 @@ def test_variable_depth_raises(test_df):
     pytest.raises(ValueError, test_df.filter, level="1/")
 
 
-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):
-    obs = test_df.filter(year=2005)
-    if test_df.time_col == "year":
-        assert obs.year == [2005]
-    else:
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-@pytest.mark.parametrize(
-    "arg_year, arg_time",
-    [
-        (dict(year=2005), dict(year=2010)),
-        (dict(time_domain="year"), dict(time_domain="datetime")),
-    ],
-)
-def test_filter_mixed_time_domain(test_df_mixed, arg_year, arg_time):
-    """Assert that reassigning attributes works for filtering from mixed time domain"""
-
-    assert test_df_mixed.time_domain == "mixed"
-
-    # filtering to datetime-only works as expected
-    obs = test_df_mixed.filter(**arg_time)
-    assert obs.time_domain == "datetime"
-    pdt.assert_index_equal(obs.time, pd.DatetimeIndex(["2010-07-21"]))
-
-    # filtering to year-only works as expected including changing of time domain
-    obs = test_df_mixed.filter(**arg_year)
-    assert obs.time_col == "year"
-    assert obs.time_domain == "year"
-    assert obs.year == [2005]
-    pdt.assert_index_equal(obs.time, pd.Int64Index([2005]))
-
-
-def test_filter_time_domain_raises(test_df_year):
-    """Assert that error is raised for invalid time_domain filter value"""
-
-    with pytest.raises(ValueError, match="Filter by `datetime='mixed'` not supported!"):
-        test_df_year.filter(time_domain="mixed")
-
-
-@pytest.mark.parametrize("test_month", [6, "June", "Jun", "jun", ["Jun", "jun"]])
-def test_filter_month(test_df, test_month):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(month=test_month).empty
-    else:
-        obs = test_df.filter(month=test_month)
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-@pytest.mark.parametrize("test_month", [6, "Jun", "jun", ["Jun", "jun"]])
-def test_filter_year_month(test_df, test_month):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(year=2005, month=test_month).empty
-    else:
-        obs = test_df.filter(year=2005, month=test_month)
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-@pytest.mark.parametrize("test_day", [17, "Fri", "Friday", "friday", ["Fri", "fri"]])
-def test_filter_day(test_df, test_day):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(day=test_day).empty
-    else:
-        obs = test_df.filter(day=test_day)
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-def test_filter_with_numpy_64_date_vals(test_df):
-    dates = test_df[test_df.time_col].unique()
-    key = "year" if test_df.time_col == "year" else "time"
-    res_0 = test_df.filter(**{key: dates[0]})
-    res = test_df.filter(**{key: dates})
-    assert np.equal(res_0.data[res_0.time_col].values, dates[0]).all()
-    assert res.equals(test_df)
-
-
-@pytest.mark.parametrize("test_hour", [0, 12, [12, 13]])
-def test_filter_hour(test_df, test_hour):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(hour=test_hour).empty
-    else:
-        obs = test_df.filter(hour=test_hour)
-        test_hour = [test_hour] if isinstance(test_hour, int) else test_hour
-        expected_rows = test_df.data["time"].apply(lambda x: x.hour).isin(test_hour)
-        expected = test_df.data["time"].loc[expected_rows].unique()
-
-        unique_time = np.array(obs["time"].unique(), dtype=np.datetime64)
-        npt.assert_array_equal(unique_time, expected)
-
-
-def test_filter_time_exact_match(test_df):
-    if test_df.time_col == "year":
-        error_msg = re.escape("Filter by `year` requires integers!")
-        with pytest.raises(TypeError, match=error_msg):
-            test_df.filter(year=datetime.datetime(2005, 6, 17))
-    else:
-        obs = test_df.filter(time=datetime.datetime(2005, 6, 17))
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-def test_filter_time_range(test_df):
-    error_msg = r".*datetime.datetime.*"
-    with pytest.raises(TypeError, match=error_msg):
-        test_df.filter(
-            year=range(datetime.datetime(2000, 6, 17), datetime.datetime(2009, 6, 17))
-        )
-
-
-def test_filter_time_range_year(test_df):
-    obs = test_df.filter(year=range(2000, 2008))
-
-    if test_df.time_col == "year":
-        assert obs.year == [2005]
-    else:
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-@pytest.mark.parametrize("month_range", [range(1, 7), "Mar-Jun"])
-def test_filter_time_range_month(test_df, month_range):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(hour=month_range).empty
-    else:
-        obs = test_df.filter(month=month_range)
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-@pytest.mark.parametrize("month_range", [["Mar-Jun", "Nov-Feb"]])
-def test_filter_time_range_round_the_clock_error(test_df, month_range):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(month=month_range).empty
-    else:
-        error_msg = re.escape(
-            "string ranges must lead to increasing integer ranges, "
-            "Nov-Feb becomes [11, 2]"
-        )
-        with pytest.raises(ValueError, match=error_msg):
-            test_df.filter(month=month_range)
-
-
-@pytest.mark.parametrize("day_range", [range(14, 20), "Thu-Sat"])
-def test_filter_time_range_day(test_df, day_range):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(day=day_range).empty
-    else:
-        obs = test_df.filter(day=day_range)
-        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
-
-
-@pytest.mark.parametrize("hour_range", [range(10, 14)])
-def test_filter_time_range_hour(test_df, hour_range):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(hour=hour_range).empty
-    else:
-        obs = test_df.filter(hour=hour_range)
-
-        expected_rows = test_df.data["time"].apply(lambda x: x.hour).isin(hour_range)
-        expected = test_df.data["time"].loc[expected_rows].unique()
-
-        unique_time = np.array(obs["time"].unique(), dtype=np.datetime64)
-        npt.assert_array_equal(unique_time, expected)
-
-
-def test_filter_time_no_match(test_df):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(time=datetime.datetime(2004, 6, 18)).empty
-    else:
-        obs = test_df.filter(time=datetime.datetime(2004, 6, 18))
-        assert obs.data.empty
-
-
-def test_filter_time_not_datetime_error(test_df):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(time=2005).empty
-    else:
-        error_msg = re.escape("`time` can only be filtered by datetimes")
-        with pytest.raises(TypeError, match=error_msg):
-            test_df.filter(time=2005)
-        with pytest.raises(TypeError, match=error_msg):
-            test_df.filter(time="summer")
-
-
-def test_filter_time_not_datetime_range_error(test_df):
-    if test_df.time_col == "year":
-        # Filtering data with yearly time domain returns empty
-        assert test_df.filter(time=range(2000, 2008)).empty
-    else:
-        error_msg = re.escape("`time` can only be filtered by datetimes")
-        with pytest.raises(TypeError, match=error_msg):
-            test_df.filter(time=range(2000, 2008))
-        with pytest.raises(TypeError, match=error_msg):
-            test_df.filter(time=["summer", "winter"])
-
-
-def test_filter_year_with_time_col(test_pd_df):
-    test_pd_df["subannual"] = ["summer", "summer", "winter"]
-    df = IamDataFrame(test_pd_df)
-    obs = df.filter(subannual="summer").timeseries()
-
-    exp = test_pd_df.set_index(IAMC_IDX + ["subannual"])
-    exp.columns = list(map(int, exp.columns))
-    pd.testing.assert_frame_equal(obs, exp[0:2])
-
-
-def test_filter_as_kwarg(test_df):
-    _df = test_df.filter(variable="Primary Energy|Coal")
-    assert _df.scenario == ["scen_a"]
-
-
-def test_filter_keep_false(test_df):
-    df = test_df.filter(variable="Primary Energy|Coal", year=2005, keep=False)
-    obs = df.data[df.data.scenario == "scen_a"].value
-    npt.assert_array_equal(obs, [1, 6, 3])
-
-
-def test_filter_by_regexp(test_df):
-    obs = test_df.filter(scenario="sce._a$", regexp=True)
-    assert obs["scenario"].unique() == "scen_a"
-
-
 def test_timeseries(test_df):
     dct = {
         "model": ["model_a"] * 2,
diff --git a/tests/test_filter.py b/tests/test_filter.py
new file mode 100644
index 0000000..9bca38c
--- /dev/null
+++ b/tests/test_filter.py
@@ -0,0 +1,260 @@
+import datetime
+import re
+import numpy as np
+import numpy.testing as npt
+import pandas as pd
+import pandas.testing as pdt
+import pytest
+
+from pyam import IamDataFrame, IAMC_IDX
+
+from conftest import EXP_DATETIME_INDEX
+
+
+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):
+    obs = test_df.filter(year=2005)
+    if test_df.time_col == "year":
+        assert obs.year == [2005]
+    else:
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+@pytest.mark.parametrize(
+    "arg_year, arg_time",
+    [
+        (dict(year=2005), dict(year=2010)),
+        (dict(time_domain="year"), dict(time_domain="datetime")),
+    ],
+)
+def test_filter_mixed_time_domain(test_df_mixed, arg_year, arg_time):
+    """Assert that reassigning attributes works for filtering from mixed time domain"""
+
+    assert test_df_mixed.time_domain == "mixed"
+
+    # filtering to datetime-only works as expected
+    obs = test_df_mixed.filter(**arg_time)
+    assert obs.time_domain == "datetime"
+    pdt.assert_index_equal(obs.time, pd.DatetimeIndex(["2010-07-21"]))
+
+    # filtering to year-only works as expected including changing of time domain
+    obs = test_df_mixed.filter(**arg_year)
+    assert obs.time_col == "year"
+    assert obs.time_domain == "year"
+    assert obs.year == [2005]
+    pdt.assert_index_equal(obs.time, pd.Int64Index([2005]))
+
+
+def test_filter_time_domain_raises(test_df_year):
+    """Assert that error is raised for invalid time_domain filter value"""
+
+    match = "Filter by `time_domain='mixed'` not supported!"
+    with pytest.raises(ValueError, match=match):
+        test_df_year.filter(time_domain="mixed")
+
+
+def test_filter_time_match_raises(test_df_time):
+    """Assert that error is raised for invalid time-component filter value"""
+
+    match = r"Could not convert months to integer: \['year'\]"
+    with pytest.raises(ValueError, match=match):
+        test_df_time.filter(month="year")
+
+
+@pytest.mark.parametrize("test_month", [6, "June", "Jun", "jun", ["Jun", "jun"]])
+def test_filter_month(test_df, test_month):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(month=test_month).empty
+    else:
+        obs = test_df.filter(month=test_month)
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+@pytest.mark.parametrize("test_month", [6, "Jun", "jun", ["Jun", "jun"]])
+def test_filter_year_month(test_df, test_month):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(year=2005, month=test_month).empty
+    else:
+        obs = test_df.filter(year=2005, month=test_month)
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+@pytest.mark.parametrize("test_day", [17, "Fri", "Friday", "friday", ["Fri", "fri"]])
+def test_filter_day(test_df, test_day):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(day=test_day).empty
+    else:
+        obs = test_df.filter(day=test_day)
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+def test_filter_with_numpy_64_date_vals(test_df):
+    dates = test_df[test_df.time_col].unique()
+    key = "year" if test_df.time_col == "year" else "time"
+    res_0 = test_df.filter(**{key: dates[0]})
+    res = test_df.filter(**{key: dates})
+    assert np.equal(res_0.data[res_0.time_col].values, dates[0]).all()
+    assert res.equals(test_df)
+
+
+@pytest.mark.parametrize("test_hour", [0, 12, [12, 13]])
+def test_filter_hour(test_df, test_hour):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(hour=test_hour).empty
+    else:
+        obs = test_df.filter(hour=test_hour)
+        test_hour = [test_hour] if isinstance(test_hour, int) else test_hour
+        expected_rows = test_df.data["time"].apply(lambda x: x.hour).isin(test_hour)
+        expected = test_df.data["time"].loc[expected_rows].unique()
+
+        unique_time = np.array(obs["time"].unique(), dtype=np.datetime64)
+        npt.assert_array_equal(unique_time, expected)
+
+
+def test_filter_time_exact_match(test_df):
+    if test_df.time_col == "year":
+        error_msg = re.escape("Filter by `year` requires integers!")
+        with pytest.raises(TypeError, match=error_msg):
+            test_df.filter(year=datetime.datetime(2005, 6, 17))
+    else:
+        obs = test_df.filter(time=datetime.datetime(2005, 6, 17))
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+def test_filter_time_range(test_df):
+    error_msg = r".*datetime.datetime.*"
+    with pytest.raises(TypeError, match=error_msg):
+        test_df.filter(
+            year=range(datetime.datetime(2000, 6, 17), datetime.datetime(2009, 6, 17))
+        )
+
+
+def test_filter_time_range_year(test_df):
+    obs = test_df.filter(year=range(2000, 2008))
+
+    if test_df.time_col == "year":
+        assert obs.year == [2005]
+    else:
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+@pytest.mark.parametrize("month_range", [range(1, 7), "Mar-Jun"])
+def test_filter_time_range_month(test_df, month_range):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(hour=month_range).empty
+    else:
+        obs = test_df.filter(month=month_range)
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+@pytest.mark.parametrize("month_range", [["Mar-Jun", "Nov-Feb"]])
+def test_filter_time_range_round_the_clock_error(test_df, month_range):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(month=month_range).empty
+    else:
+        error_msg = re.escape(
+            "string ranges must lead to increasing integer ranges, "
+            "Nov-Feb becomes [11, 2]"
+        )
+        with pytest.raises(ValueError, match=error_msg):
+            test_df.filter(month=month_range)
+
+
+@pytest.mark.parametrize("day_range", [range(14, 20), "Thu-Sat"])
+def test_filter_time_range_day(test_df, day_range):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(day=day_range).empty
+    else:
+        obs = test_df.filter(day=day_range)
+        pdt.assert_index_equal(obs.time, EXP_DATETIME_INDEX)
+
+
+@pytest.mark.parametrize("hour_range", [range(10, 14)])
+def test_filter_time_range_hour(test_df, hour_range):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(hour=hour_range).empty
+    else:
+        obs = test_df.filter(hour=hour_range)
+
+        expected_rows = test_df.data["time"].apply(lambda x: x.hour).isin(hour_range)
+        expected = test_df.data["time"].loc[expected_rows].unique()
+
+        unique_time = np.array(obs["time"].unique(), dtype=np.datetime64)
+        npt.assert_array_equal(unique_time, expected)
+
+
+def test_filter_time_no_match(test_df):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(time=datetime.datetime(2004, 6, 18)).empty
+    else:
+        obs = test_df.filter(time=datetime.datetime(2004, 6, 18))
+        assert obs.data.empty
+
+
+def test_filter_time_not_datetime_error(test_df):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(time=2005).empty
+    else:
+        error_msg = re.escape("`time` can only be filtered by datetimes")
+        with pytest.raises(TypeError, match=error_msg):
+            test_df.filter(time=2005)
+        with pytest.raises(TypeError, match=error_msg):
+            test_df.filter(time="summer")
+
+
+def test_filter_time_not_datetime_range_error(test_df):
+    if test_df.time_col == "year":
+        # Filtering data with yearly time domain returns empty
+        assert test_df.filter(time=range(2000, 2008)).empty
+    else:
+        error_msg = re.escape("`time` can only be filtered by datetimes")
+        with pytest.raises(TypeError, match=error_msg):
+            test_df.filter(time=range(2000, 2008))
+        with pytest.raises(TypeError, match=error_msg):
+            test_df.filter(time=["summer", "winter"])
+
+
+def test_filter_year_with_time_col(test_pd_df):
+    test_pd_df["subannual"] = ["summer", "summer", "winter"]
+    df = IamDataFrame(test_pd_df)
+    obs = df.filter(subannual="summer").timeseries()
+
+    exp = test_pd_df.set_index(IAMC_IDX + ["subannual"])
+    exp.columns = list(map(int, exp.columns))
+    pd.testing.assert_frame_equal(obs, exp[0:2])
+
+
+def test_filter_as_kwarg(test_df):
+    _df = test_df.filter(variable="Primary Energy|Coal")
+    assert _df.scenario == ["scen_a"]
+
+
+def test_filter_keep_false(test_df):
+    df = test_df.filter(variable="Primary Energy|Coal", year=2005, keep=False)
+    obs = df.data[df.data.scenario == "scen_a"].value
+    npt.assert_array_equal(obs, [1, 6, 3])
+
+
+def test_filter_by_regexp(test_df):
+    obs = test_df.filter(scenario="sce._a$", regexp=True)
+    assert obs["scenario"].unique() == "scen_a"