Merge pull request #425 from NREL/new_default_behavior

Update default behaviors

.github/workflows/pytest.yaml

@@ -30,6 +30,7 @@ jobs:
           # required numpy is not available on python 3.7, so exclude:
           - python-version: 3.7
             env: '-r requirements.txt .[test]'
+      fail-fast: false
 
     steps:
     - uses: actions/checkout@v2

docs/notebook_requirements.txt

@@ -16,7 +16,7 @@ ipython-genutils==0.2.0
 ipywidgets==7.3.0
 jedi==0.16.0
 Jinja2==3.1.3
-jsonschema==2.6.0
+jsonschema==4.18.0
 jupyter==1.0.0
 jupyter-client==8.6.1
 jupyter-console==6.6.3
@@ -27,22 +27,22 @@ MarkupSafe==2.0.0
 mistune==2.0.3
 nbclient==0.6.3
 nbconvert==7.0.0
-nbformat==5.1.0
+nbformat==5.3.0
 nest-asyncio==1.5.5
 notebook==6.4.12
 numexpr==2.10.0
 pandocfilters==1.5.1
 parso==0.5.2
 pexpect==4.6.0
 pickleshare==0.7.5
-prometheus-client==0.3.0
+prometheus-client==0.9
 prompt-toolkit==3.0.43
 ptyprocess==0.6.0
 pycparser==2.20
 Pygments==2.15.0
 pyzmq==26.0.2
 qtconsole==4.3.1
-Send2Trash==1.8.0
+Send2Trash==1.8.2
 simplegeneric==0.8.1
 soupsieve==2.3.2.post1
 terminado==0.8.3

docs/sphinx/source/changelog/pending.rst

@@ -9,6 +9,11 @@ when compared with older versions of RdTools
 
 * Use the pvlib method for clear sky detection by default in :py:func:`~rdtools.analysis_chains.TrendAnalysis` (:pull:`412`)
 
+* Change default clipping filter `model` to `logic` (:pull:`425`)
+
+* Turn on the `two_way_window_filter` by default in `TrendAnalysis`
+(:pull:`425`)
+
 Enhancements
 ------------
 * Added a new wrapper function for clearsky filters (:pull:`412`)

rdtools/analysis_chains.py

@@ -157,7 +157,10 @@ def __init__(
             "clearsky_filter": {},
             "ad_hoc_filter": None,  # use this to include an explict filter
         }
-        self.filter_params_aggregated = {"ad_hoc_filter": None}
+        self.filter_params_aggregated = {
+            "two_way_window_filter": {},
+            "ad_hoc_filter": None
+        }
         # remove tcell_filter from list if power_expected is passed in
         if power_expected is not None and temperature_cell is None:
             del self.filter_params["tcell_filter"]

rdtools/filtering.py

@@ -5,7 +5,6 @@
 import os
 import warnings
 import pvlib
-from numbers import Number
 from scipy.interpolate import interp1d
 import rdtools
 import xgboost as xgb
@@ -292,7 +291,7 @@ def pvlib_clearsky_filter(
     return mask
 
 
-def clip_filter(power_ac, model="quantile", **kwargs):
+def clip_filter(power_ac, model="logic", **kwargs):
     """
     Master wrapper for running one of the desired clipping filters.
     The default filter run is the quantile clipping filter.
@@ -302,7 +301,7 @@ def clip_filter(power_ac, model="quantile", **kwargs):
     power_ac : pandas.Series
         Pandas time series, representing PV system power or energy.
         For best performance, timestamps should be in local time.
-    model : str, default 'quantile'
+    model : str, default 'logic'
         Clipping filter model to run. Can be 'quantile',
         'xgboost', or 'logic'. Note: using the xgboost model can
         result in errors on some systems. These can often be alleviated
@@ -320,18 +319,6 @@ def clip_filter(power_ac, model="quantile", **kwargs):
         True values delineate non-clipping periods, and False values delineate
         clipping periods.
     """
-    if isinstance(model, Number):
-        quantile = model
-        warnings.warn(
-            "Function clip_filter is now a wrapper for different "
-            "clipping filters. To reproduce prior behavior, "
-            "parameters have been interpreted as model= "
-            f"'quantile_clip_filter', quantile={quantile}. "
-            "This syntax will be removed in a future version.",
-            rdtools._deprecation.rdtoolsDeprecationWarning,
-        )
-        kwargs["quantile"] = quantile
-        model = "quantile"
 
     if model == "quantile":
         clip_mask = quantile_clip_filter(power_ac, **kwargs)

rdtools/test/analysis_chains_test.py

@@ -155,6 +155,15 @@ def test_sensor_analysis(sensor_analysis):
     assert [-1, -1] == pytest.approx(ci, abs=1e-2)
 
 
+def test_sensor_analysis_filter_components(sensor_analysis):
+    columns = sensor_analysis.sensor_filter_components_aggregated.columns
+    assert {'two_way_window_filter'} == set(columns)
+
+    expected_columns = {'normalized_filter', 'poa_filter', 'tcell_filter', 'clip_filter'}
+    columns = sensor_analysis.sensor_filter_components.columns
+    assert expected_columns == set(columns)
+
+
 def test_sensor_analysis_energy(sensor_parameters, sensor_analysis):
     sensor_parameters["pv"] = sensor_analysis.pv_energy
     sensor_parameters["pv_input"] = "energy"
@@ -214,7 +223,7 @@ def test_sensor_analysis_aggregated_ad_hoc_filter(sensor_parameters):
         rd_analysis.sensor_analysis(analyses=["yoy_degradation"])
 
 
-def test_filter_components(sensor_parameters):
+def test_filter_components_poa(sensor_parameters):
     poa = sensor_parameters["poa_global"]
     poa_filter = (poa > 200) & (poa < 1200)
     rd_analysis = TrendAnalysis(**sensor_parameters, power_dc_rated=1.0)
@@ -482,8 +491,18 @@ def test_clearsky_analysis(clearsky_analysis):
     ci = yoy_results["rd_confidence_interval"]
     rd = yoy_results["p50_rd"]
     print(ci)
-    assert -4.70 == pytest.approx(rd, abs=1e-2)
-    assert [-4.71, -4.69] == pytest.approx(ci, abs=1e-2)
+    assert pytest.approx(rd, abs=1e-2) == -5.15
+    assert pytest.approx(ci, abs=1e-2) == [-5.17, -5.13]
+
+
+def test_clearsky_analysis_filter_components(clearsky_analysis):
+    columns = clearsky_analysis.clearsky_filter_components_aggregated.columns
+    assert {'two_way_window_filter'} == set(columns)
+
+    expected_columns = {'normalized_filter', 'poa_filter', 'tcell_filter',
+                        'clip_filter', 'clearsky_filter'}
+    columns = clearsky_analysis.clearsky_filter_components.columns
+    assert expected_columns == set(columns)
 
 
 def test_clearsky_analysis_optional(
@@ -496,8 +515,8 @@ def test_clearsky_analysis_optional(
     ci = yoy_results["rd_confidence_interval"]
     rd = yoy_results["p50_rd"]
     print(f"ci:{ci}")
-    assert -4.70 == pytest.approx(rd, abs=1e-2)
-    assert [-4.71, -4.69] == pytest.approx(ci, abs=1e-2)
+    assert pytest.approx(rd, abs=1e-2) == -5.15
+    assert pytest.approx(ci, abs=1e-2) == [-5.17, -5.13]
 
 
 def test_sensor_clearsky_analysis(sensor_clearsky_analysis):
@@ -632,9 +651,9 @@ def test_srr_soiling(soiling_analysis_sensor):
     assert [0.96, 0.97] == pytest.approx(
         ci, abs=1e-2
     ), "Soiling confidence interval different from expected value in TrendAnalysis.srr_soiling"
-    assert 0.974 == pytest.approx(
+    assert pytest.approx(
         renorm_factor, abs=1e-3
-    ), "Renormalization factor different from expected value in TrendAnalysis.srr_soiling"
+    ) == 0.977, "Renormalization factor different from expected value in TrendAnalysis.srr_soiling"
 
 
 def test_plot_degradation(sensor_analysis):

rdtools/test/filtering_test.py

@@ -20,6 +20,7 @@
                      hour_angle_filter)
 import warnings
 from conftest import assert_warnings
+from pandas import testing as tm
 
 
 def test_clearsky_filter(mocker):
@@ -321,50 +322,87 @@ def test_xgboost_clip_filter(generate_power_time_series_no_clipping,
                 .all(axis=None))
 
 
-def test_clip_filter(generate_power_time_series_no_clipping):
+def test_clip_filter(generate_power_time_series_clipping, mocker):
     ''' Unit tests for inverter clipping filter.'''
     # Create a time series to test
-    power_no_datetime_index_nc, power_datetime_index_nc, power_nc_tz_naive = \
-        generate_power_time_series_no_clipping
-    # Check that the master wrapper defaults to the
-    # quantile_clip_filter_function.
-    # Note: Power is expected to be Series object because clip_filter makes
-    #       use of the Series.quantile() method.
-    filtered_quantile = clip_filter(power_no_datetime_index_nc, quantile=0.98)
-    # Expect 99% of the 98th quantile to be filtered
-    expected_result_quantile = power_no_datetime_index_nc < (98 * 0.99)
-    # Check that the clip filter defaults to quantile clip filter when
-    # deprecated params are passed
-    warnings.simplefilter("always")
-    with warnings.catch_warnings(record=True) as record:
-        clip_filter(power_datetime_index_nc, 0.98)
-        assert_warnings(['Function clip_filter is now a wrapper'], record)
+    _, power = generate_power_time_series_clipping
+
+    # Check the default behavior
+    expected = logic_clip_filter(power)
+    mock_logic_clip_filter = mocker.patch('rdtools.filtering.logic_clip_filter',
+                                          return_value=expected)
+    filtered = clip_filter(power)
+    mock_logic_clip_filter.assert_called_once()
+    tm.assert_series_equal(filtered, expected)
+
+    # Check each of the models
+    expected_kwargs = {
+        'mounting_type': 'single_axis_tracking',
+        'rolling_range_max_cutoff': 0.3,
+        'roll_periods': 3
+    }
+    expected = logic_clip_filter(power, **expected_kwargs)
+    mock_logic_clip_filter = mocker.patch('rdtools.filtering.logic_clip_filter',
+                                          return_value=expected)
+    filtered = clip_filter(power, model='logic', **expected_kwargs)
+    mock_logic_clip_filter.assert_called_once()
+    call_args = mock_logic_clip_filter.call_args
+
+    # Deal with change in call_args after python 3.7
+    if isinstance(call_args, tuple):  # case for 3.7
+        actual_kwargs = call_args[1]
+    else:
+        actual_kwargs = call_args.kwargs
+
+    assert actual_kwargs == expected_kwargs
+    tm.assert_series_equal(filtered, expected)
+
+    expected_kwargs = {
+        'quantile': 0.95
+    }
+    expected = quantile_clip_filter(power, **expected_kwargs)
+    mock_quantile_clip_filter = mocker.patch('rdtools.filtering.quantile_clip_filter',
+                                             return_value=expected)
+    filtered = clip_filter(power, model='quantile', **expected_kwargs)
+    mock_quantile_clip_filter.assert_called_once()
+    call_args = mock_quantile_clip_filter.call_args
+    if isinstance(call_args, tuple):
+        actual_kwargs = call_args[1]
+    else:
+        actual_kwargs = call_args.kwargs
+
+    assert actual_kwargs == expected_kwargs
+    tm.assert_series_equal(filtered, expected)
+
+    expected_kwargs = {
+        'mounting_type': 'single_axis_tracking'
+    }
+    expected = xgboost_clip_filter(power, **expected_kwargs)
+    mock_xgboost_clip_filter = mocker.patch('rdtools.filtering.xgboost_clip_filter',
+                                            return_value=expected)
+    filtered = clip_filter(power, model='xgboost', **expected_kwargs)
+    mock_xgboost_clip_filter.assert_called_once()
+    call_args = mock_xgboost_clip_filter.call_args
+    if isinstance(call_args, tuple):
+        actual_kwargs = call_args[1]
+    else:
+        actual_kwargs = call_args.kwargs
+    assert actual_kwargs == expected_kwargs
+    tm.assert_series_equal(filtered, expected)
+
+    mocker.stopall()
 
     # Check that a ValueError is thrown when a model is passed that
     # is not in the acceptable list.
     with pytest.raises(ValueError):
-        clip_filter(power_datetime_index_nc, 'random_forest')
-    # Check that the wrapper handles the xgboost clipping
-    # function with kwargs.
-    filtered_xgboost = clip_filter(power_datetime_index_nc,
-                                   'xgboost',
-                                   mounting_type="fixed")
-    # Check that the wrapper handles the logic clipping
-    # function with kwargs.
-    filtered_logic = clip_filter(power_datetime_index_nc,
-                                 'logic',
-                                 mounting_type="fixed",
-                                 rolling_range_max_cutoff=0.3)
-    # Check that the function returns a Typr Error if a wrong keyword
+        clip_filter(power, 'random_forest')
+
+    # Check that the function returns a Type Error if a wrong keyword
     # arg is passed in the kwarg arguments.
     with pytest.raises(TypeError):
-        clip_filter(power_datetime_index_nc,
+        clip_filter(power,
                     'xgboost',
                     rolling_range_max_cutoff=0.3)
-    assert bool((expected_result_quantile == filtered_quantile)
-                .all(axis=None))
-    assert bool(filtered_xgboost.all(axis=None))
-    assert bool(filtered_logic.all(axis=None))
 
 
 def test_normalized_filter_default():

requirements.txt

@@ -9,7 +9,7 @@ joblib==1.2.0
 kiwisolver==1.3.2
 matplotlib==3.5.0
 numpy==1.22.0
-packaging==21.3
+packaging==22.0
 pandas==1.3.4
 patsy==0.5.2
 Pillow==10.3.0