Repository: pvlib/pvanalytics

Issue:
stale_values_diff and interpolation_diff : option to label all values
Consider adding an option to label all values in a stale / interpolation window as stale, rather than only labelling the end value in a window.

Discussion:

Patch:
diff --git a/pvanalytics/quality/gaps.py b/pvanalytics/quality/gaps.py
index ff872da..82c1ddc 100644
--- a/pvanalytics/quality/gaps.py
+++ b/pvanalytics/quality/gaps.py
@@ -51,7 +51,17 @@ def _backfill_window(endpoints, window):
     return flags
 
 
-def stale_values_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
+def _mark(flags, window, mark):
+    if mark not in ['all', 'tail', 'end']:
+        raise ValueError("mark must be one of'tail', 'end' or 'all'")
+    if mark == 'all':
+        return _backfill_window(flags, window)
+    if mark == 'tail':
+        return _backfill_window(flags, window - 1)
+    return flags
+
+
+def stale_values_diff(x, window=6, rtol=1e-5, atol=1e-8, mark='tail'):
     """Identify stale values in the data.
 
     For a window of length N, the last value (index N-1) is considered
@@ -72,9 +82,17 @@ def stale_values_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
         relative tolerance for detecting a change in data values
     atol : float, default 1e-8
         absolute tolerance for detecting a change in data values
-    label_all : bool, default False
-        Whether to label all values in the window. If False, then only
-        the right endpoint of the window is labeled.
+    mark : str, default 'tail'
+        How much of the window to mark ``True`` when a sequence of
+        stale values is detected. Can be of 'tail', 'end', or 'all'.
+
+        - If 'tail' (the default) then every point in the window
+          *except* the first point is marked ``True``.
+        - If 'end' then only the endpoints of the window are marked
+          ``True``. The first `window - 1` values in a stale sequence
+          sequence are marked ``False``.
+        - If 'all' then every point in the window *including* the
+          first point is marked ``True``.
 
     Returns
     -------
@@ -84,7 +102,8 @@ def stale_values_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
     Raises
     ------
     ValueError
-        If window < 2.
+        If `window < 2` or `mark` is not one of 'tail', 'end', or
+        'all'.
 
     Notes
     -----
@@ -103,12 +122,10 @@ def stale_values_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
         raw=True,
         kwargs={'rtol': rtol, 'atol': atol}
     ).fillna(False).astype(bool)
-    if label_all:
-        return _backfill_window(flags, window)
-    return flags
+    return _mark(flags, window, mark)
 
 
-def interpolation_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
+def interpolation_diff(x, window=6, rtol=1e-5, atol=1e-8, mark='tail'):
     """Identify sequences which appear to be linear.
 
     Sequences are linear if the first difference appears to be
@@ -129,9 +146,18 @@ def interpolation_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
         tolerance relative to max(abs(x.diff()) for detecting a change
     atol : float, default 1e-8
         absolute tolerance for detecting a change in first difference
-    label_all : bool, default False
-        Whether to label all values in the window. If False, then only the
-        right endpoint of the window is labeled.
+    mark : str, default 'tail'
+        How much of the window to mark ``True`` when a sequence of
+        interpolated values is detected. Can be 'tail', 'end', or
+        'all'.
+
+        - If 'tail' (the default) then every point in the window
+          *except* the first point is marked ``True``.
+        - If 'end' then only the endpoints of the window are marked
+          ``True``. The first `window - 1` values in an interpolated
+          sequence are marked ``False``.
+        - If 'all' then every point in the window *including* the
+          first point is marked ``True``.
 
     Returns
     -------
@@ -141,7 +167,8 @@ def interpolation_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
     Raises
     ------
     ValueError
-        If window < 3.
+        If `window < 3` or `mark` is not one of 'tail', 'end', or
+        'all'.
 
     Notes
     -----
@@ -160,11 +187,10 @@ def interpolation_diff(x, window=6, rtol=1e-5, atol=1e-8, label_all=False):
         x.diff(periods=1),
         window=window-1,
         rtol=rtol,
-        atol=atol
+        atol=atol,
+        mark='end'
     )
-    if label_all:
-        return _backfill_window(flags, window)
-    return flags
+    return _mark(flags, window, mark)
 
 
 def _freq_to_seconds(freq):


Test patch:
diff --git a/pvanalytics/tests/quality/test_gaps.py b/pvanalytics/tests/quality/test_gaps.py
index 6c27558..7896b29 100644
--- a/pvanalytics/tests/quality/test_gaps.py
+++ b/pvanalytics/tests/quality/test_gaps.py
@@ -52,14 +52,14 @@ def test_stale_values_diff(stale_data):
     for more information.
 
     """
-    res0 = gaps.stale_values_diff(stale_data)
-    res1 = gaps.stale_values_diff(stale_data, window=3)
-    res2 = gaps.stale_values_diff(stale_data, rtol=1e-8, window=2)
-    res3 = gaps.stale_values_diff(stale_data, window=7)
-    res4 = gaps.stale_values_diff(stale_data, window=8)
-    res5 = gaps.stale_values_diff(stale_data, rtol=1e-8, window=4)
-    res6 = gaps.stale_values_diff(stale_data[1:], window=3)
-    res7 = gaps.stale_values_diff(stale_data[1:8], window=3)
+    res0 = gaps.stale_values_diff(stale_data, mark='end')
+    res1 = gaps.stale_values_diff(stale_data, window=3, mark='end')
+    res2 = gaps.stale_values_diff(stale_data, rtol=1e-8, window=2, mark='end')
+    res3 = gaps.stale_values_diff(stale_data, window=7, mark='end')
+    res4 = gaps.stale_values_diff(stale_data, window=8, mark='end')
+    res5 = gaps.stale_values_diff(stale_data, rtol=1e-8, window=4, mark='end')
+    res6 = gaps.stale_values_diff(stale_data[1:], window=3, mark='end')
+    res7 = gaps.stale_values_diff(stale_data[1:8], window=3, mark='end')
     assert_series_equal(res0, pd.Series([False, False, False, False, False,
                                          False, True, True, False, False]))
     assert_series_equal(res1, pd.Series([False, False, False, True, True, True,
@@ -91,16 +91,22 @@ def test_stale_values_diff_handles_negatives(data_with_negatives):
     for more information.
 
     """
-    res = gaps.stale_values_diff(data_with_negatives, window=3)
+    res = gaps.stale_values_diff(data_with_negatives, window=3, mark='end')
     assert_series_equal(res, pd.Series([False, False, True, True, False, False,
                                         False]))
-    res = gaps.stale_values_diff(data_with_negatives, window=3, atol=1e-3)
+    res = gaps.stale_values_diff(
+        data_with_negatives, window=3, atol=1e-3, mark='end'
+    )
     assert_series_equal(res, pd.Series([False, False, True, True, True, True,
                                         True]))
-    res = gaps.stale_values_diff(data_with_negatives, window=3, atol=1e-5)
+    res = gaps.stale_values_diff(
+        data_with_negatives, window=3, atol=1e-5, mark='end'
+    )
     assert_series_equal(res, pd.Series([False, False, True, True, True, False,
                                         False]))
-    res = gaps.stale_values_diff(data_with_negatives, window=3, atol=2e-5)
+    res = gaps.stale_values_diff(
+        data_with_negatives, window=3, atol=2e-5, mark='end'
+    )
     assert_series_equal(res, pd.Series([False, False, True, True, True, True,
                                         True]))
 
@@ -121,14 +127,28 @@ def test_stale_values_diff_raises_error(stale_data):
         gaps.stale_values_diff(stale_data, window=1)
 
 
-def test_stale_values_diff_label_all(stale_data):
-    """When label_all is True the full window is marked stale"""
+def test_stale_values_diff_raises_error_for_bad_mark(stale_data):
+    """Passing mark not in ['all', 'end', 'tail'] raises a ValueError."""
+    with pytest.raises(ValueError):
+        gaps.stale_values_diff(stale_data, mark='head')
+
+
+def test_stale_values_diff_mark_all(stale_data):
+    """When mark='all' the full window is marked stale"""
     assert_series_equal(
         pd.Series([False, True, True, True, True,
                    True, True, True, False, False]),
-        gaps.stale_values_diff(
-            stale_data, window=4, label_all=True
-        )
+        gaps.stale_values_diff(stale_data, window=4, mark='all')
+    )
+
+
+def test_stale_values_diff_mark_tail(stale_data):
+    """When mark='tail' (the default), every point in the window except
+    the first is marked stale."""
+    assert_series_equal(
+        pd.Series([False, False, True, True, True,
+                   True, True, True, False, False]),
+        gaps.stale_values_diff(stale_data, window=4)
     )
 
 
@@ -150,10 +170,10 @@ def interpolated_data():
     return pd.Series(data=data)
 
 
-def test_interpolation_diff_label_all(interpolated_data):
-    """When label_all is True the full window is marked interpoated"""
+def test_interpolation_diff_mark_all(interpolated_data):
+    """When mark='all' the full window is marked interpoated"""
     assert_series_equal(
-        gaps.interpolation_diff(interpolated_data, window=3, label_all=True),
+        gaps.interpolation_diff(interpolated_data, window=3, mark='all'),
         pd.Series([False, False, False, False, False,
                    True, True, True, False, False,
                    False, True, True, True, True, True,
@@ -161,6 +181,18 @@ def test_interpolation_diff_label_all(interpolated_data):
     )
 
 
+def test_interpolation_diff_mark_tail(interpolated_data):
+    """When mark='tail' (the default), all but the first point an the
+    window is marked interpolated."""
+    assert_series_equal(
+        gaps.interpolation_diff(interpolated_data, window=3),
+        pd.Series([False, False, False, False, False,
+                   False, True, True, False, False,
+                   False, False, True, True, True, True,
+                   False])
+    )
+
+
 def test_interpolation_diff(interpolated_data):
     """Interpolation is detected correclty.
 
@@ -173,27 +205,31 @@ def test_interpolation_diff(interpolated_data):
     for more information.
 
     """
-    res0 = gaps.interpolation_diff(interpolated_data)
+    res0 = gaps.interpolation_diff(interpolated_data, mark='end')
     assert_series_equal(res0, pd.Series([False, False, False, False, False,
                                          False, False, False, False, False,
                                          False, False, False, False, False,
                                          False, False]))
-    res1 = gaps.interpolation_diff(interpolated_data, window=3)
+    res1 = gaps.interpolation_diff(interpolated_data, window=3, mark='end')
     assert_series_equal(res1, pd.Series([False, False, False, False, False,
                                          False, False, True, False, False,
                                          False, False, False, True, True, True,
                                          False]))
-    res2 = gaps.interpolation_diff(interpolated_data, window=3, rtol=1e-2)
+    res2 = gaps.interpolation_diff(
+        interpolated_data, window=3, rtol=1e-2, mark='end'
+    )
     assert_series_equal(res2, pd.Series([False, False, True, True, True,
                                          False, False, True, False, False,
                                          False, False, False, True, True, True,
                                          False]))
-    res3 = gaps.interpolation_diff(interpolated_data, window=5)
+    res3 = gaps.interpolation_diff(interpolated_data, window=5, mark='end')
     assert_series_equal(res3, pd.Series([False, False, False, False, False,
                                          False, False, False, False, False,
                                          False, False, False, False, False,
                                          True, False]))
-    res4 = gaps.interpolation_diff(interpolated_data, window=3, atol=1e-2)
+    res4 = gaps.interpolation_diff(
+        interpolated_data, window=3, atol=1e-2, mark='end'
+    )
     assert_series_equal(res4, pd.Series([False, False, True, True, True,
                                          True, True, True, False, False,
                                          False, False, False, True, True, True,
@@ -212,10 +248,14 @@ def test_interpolation_diff_handles_negatives(data_with_negatives):
     for more information.
 
     """
-    res = gaps.interpolation_diff(data_with_negatives, window=3, atol=1e-5)
+    res = gaps.interpolation_diff(
+        data_with_negatives, window=3, atol=1e-5, mark='end'
+    )
     assert_series_equal(res, pd.Series([False, False, True, True, True, True,
                                         False]))
-    res = gaps.stale_values_diff(data_with_negatives, window=3, atol=1e-4)
+    res = gaps.stale_values_diff(
+        data_with_negatives, window=3, atol=1e-4, mark='end'
+    )
     assert_series_equal(res, pd.Series([False, False, True, True, True, True,
                                         True]))
 
@@ -233,7 +273,13 @@ def test_interpolation_diff_raises_error(interpolated_data):
 
     """
     with pytest.raises(ValueError):
-        gaps.interpolation_diff(interpolated_data, window=2)
+        gaps.interpolation_diff(interpolated_data, window=2, mark='end')
+
+
+def test_interpolation_diff_raises_error_for_bad_mark(interpolated_data):
+    """Passing mark not in ['all', 'end', 'tail'] raises a ValueError."""
+    with pytest.raises(ValueError):
+        gaps.interpolation_diff(interpolated_data, mark='bad')
 
 
 def test_start_stop_dates_all_true():