Merge pull request #436 from NREL/fix_issue_#377

Add checks to filter arguments for TrendAnalysis (Issue #377)

Author: mdeceglie

docs/sphinx/source/changelog/pending.rst

@@ -5,6 +5,7 @@ v3.0.0 (December XX, 2024)
 Enhancements
 ------------
 * Add `CITATION.cff` file for citation information (:pull:`434`)
+* Added checks to TrendAnalysis for `filter_params` and `filter_params_aggregated`. Raises an error if unkown filter is supplied. (:pull:`436`)
 
 
 Bug fixes

rdtools/analysis_chains.py

@@ -155,23 +155,80 @@ def __init__(
         self.max_timedelta = max_timedelta
         self.results = {}
 
-        # Initialize to use default filter parameters
-        self.filter_params = {
+        # Define valid filter parameters
+        self.valid_filter_params = [
+            "normalized_filter",
+            "poa_filter",
+            "tcell_filter",
+            "clip_filter",
+            "hour_angle_filter",
+            "clearsky_filter",
+            "sensor_clearsky_filter",
+            "ad_hoc_filter",
+        ]
+
+        self.valid_filter_params_aggregated = [
+            "two_way_window_filter",
+            "insolation_filter",
+            "hampel_filter",
+            "directional_tukey_filter",
+            "ad_hoc_filter",
+        ]
+
+        # Define default filter parameters
+        self.default_filter_params = {
             "normalized_filter": {},
             "poa_filter": {},
             "tcell_filter": {},
             "clip_filter": {},
             "clearsky_filter": {},
             "ad_hoc_filter": None,  # use this to include an explict filter
         }
-        self.filter_params_aggregated = {
+
+        self.default_filter_params_aggregated = {
             "two_way_window_filter": {},
-            "ad_hoc_filter": None
+            "ad_hoc_filter": None,
         }
+
+        # Initialize to use default filter parameters
+        self._filter_params = ValidatedFilterDict(
+            self.valid_filter_params, self.default_filter_params
+        )
+        self._filter_params_aggregated = ValidatedFilterDict(
+            self.valid_filter_params_aggregated, self.default_filter_params_aggregated
+        )
         # 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"]
 
+    @property
+    def filter_params(self):
+        return self._filter_params
+
+    @filter_params.setter
+    def filter_params(self, new_filter_params):
+        if not isinstance(new_filter_params, dict):
+            raise ValueError("Attribute `filter_params` must be a dictionary.")
+
+        # If dictionary passed, check the new filter_params and set new filters.
+        self._filter_params = ValidatedFilterDict(self.valid_filter_params, new_filter_params)
+        print(f"Attribute `filter_params` changed to: {new_filter_params}")
+
+    @property
+    def filter_params_aggregated(self):
+        return self._filter_params_aggregated
+
+    @filter_params_aggregated.setter
+    def filter_params_aggregated(self, new_filter_params_aggregated):
+        if not (isinstance(new_filter_params_aggregated, dict) or None):
+            raise ValueError("Attribute `filter_params_aggregated` must be a dictionary.")
+
+        # If dictionary passed, check the new filter_params and set new filters.
+        self._filter_params_aggregated = ValidatedFilterDict(
+            self.valid_filter_params_aggregated, new_filter_params_aggregated
+        )
+        print(f"Attribute `filter_params_aggregated` changed to: {new_filter_params_aggregated}")
+
     def set_clearsky(
         self,
         pvlib_location=None,
@@ -1205,3 +1262,27 @@ def plot_degradation_timeseries(self, case, rolling_days=365, **kwargs):
 
         fig = plotting.degradation_timeseries_plot(yoy_info, rolling_days, **kwargs)
         return fig
+
+
+class ValidatedFilterDict(dict):
+    def __init__(self, valid_keys, *args, **kwargs):
+        self.valid_keys = valid_keys
+        self._err_msg = "Key '{0}' is not a valid filter parameter."
+        super(ValidatedFilterDict, self).__init__(*args, **kwargs)
+        self._validate_keys()
+
+    def __setitem__(self, key, value):
+        if key not in self.valid_keys:
+            raise KeyError(self._err_msg.format(key))
+        super(ValidatedFilterDict, self).__setitem__(key, value)
+
+    def update(self, *args, **kwargs):
+        for key in dict(*args, **kwargs).keys():
+            if key not in self.valid_keys:
+                raise KeyError(self._err_msg.format(key))
+        super(ValidatedFilterDict, self).update(*args, **kwargs)
+
+    def _validate_keys(self):
+        for key in self.keys():
+            if key not in self.valid_keys:
+                raise KeyError(self._err_msg.format(key))

rdtools/test/analysis_chains_test.py

@@ -1,5 +1,6 @@
 from rdtools import TrendAnalysis, normalization, filtering
 from conftest import assert_isinstance, assert_warnings
+from rdtools.analysis_chains import ValidatedFilterDict
 import pytest
 import pvlib
 import pandas as pd
@@ -740,3 +741,92 @@ def test_plot_degradation_timeseries(sensor_analysis, clearsky_analysis):
     assert_isinstance(
         clearsky_analysis.plot_degradation_timeseries("clearsky"), plt.Figure
     )
+
+
+def test_validated_filter_dict_initialization():
+    valid_keys = ["key1", "key2"]
+    filter_dict = ValidatedFilterDict(valid_keys, key1="value1", key2="value2")
+    assert filter_dict["key1"] == "value1"
+    assert filter_dict["key2"] == "value2"
+
+
+def test_validated_filter_dict_invalid_key_initialization():
+    valid_keys = ["key1", "key2"]
+    with pytest.raises(KeyError, match="Key 'key3' is not a valid filter parameter."):
+        ValidatedFilterDict(valid_keys, key1="value1", key3="value3")
+
+
+def test_validated_filter_dict_setitem():
+    valid_keys = ["key1", "key2"]
+    filter_dict = ValidatedFilterDict(valid_keys)
+    filter_dict["key1"] = "value1"
+    assert filter_dict["key1"] == "value1"
+
+
+def test_validated_filter_dict_setitem_invalid_key():
+    valid_keys = ["key1", "key2"]
+    filter_dict = ValidatedFilterDict(valid_keys)
+    with pytest.raises(KeyError, match="Key 'key3' is not a valid filter parameter."):
+        filter_dict["key3"] = "value3"
+
+
+def test_validated_filter_dict_update():
+    valid_keys = ["key1", "key2"]
+    filter_dict = ValidatedFilterDict(valid_keys)
+    filter_dict.update({"key1": "value1", "key2": "value2"})
+    assert filter_dict["key1"] == "value1"
+    assert filter_dict["key2"] == "value2"
+
+
+def test_validated_filter_dict_update_invalid_key():
+    valid_keys = ["key1", "key2"]
+    filter_dict = ValidatedFilterDict(valid_keys)
+    with pytest.raises(KeyError, match="Key 'key3' is not a valid filter parameter."):
+        filter_dict.update({"key1": "value1", "key3": "value3"})
+
+
+@pytest.mark.parametrize(
+    "filter_param",
+    [
+        "normalized_filter",
+        "poa_filter",
+        "tcell_filter",
+        "clip_filter",
+        "hour_angle_filter",
+        "clearsky_filter",
+        "sensor_clearsky_filter",
+        "ad_hoc_filter",
+    ],
+)
+def test_valid_filter_params(sensor_analysis, filter_param):
+    sensor_analysis.filter_params[filter_param] = {}
+    assert filter_param in sensor_analysis.filter_params
+
+
+def test_invalid_filter_params(sensor_analysis, filter_param="invalid_filter"):
+    with pytest.raises(KeyError, match=f"Key '{filter_param}' is not a valid filter parameter."):
+        sensor_analysis.filter_params[filter_param] = {}
+
+
+@pytest.mark.parametrize(
+    "filter_param_aggregated",
+    [
+        "two_way_window_filter",
+        "insolation_filter",
+        "hampel_filter",
+        "directional_tukey_filter",
+        "ad_hoc_filter",
+    ],
+)
+def test_valid_filter_params_aggregated(sensor_analysis, filter_param_aggregated):
+    sensor_analysis.filter_params_aggregated[filter_param_aggregated] = {}
+    assert filter_param_aggregated in sensor_analysis.filter_params_aggregated
+
+
+def test_invalid_filter_params_aggregated(
+    sensor_analysis, filter_param_aggregated="invalid_filter"
+):
+    with pytest.raises(
+        KeyError, match=f"Key '{filter_param_aggregated}' is not a valid filter parameter."
+    ):
+        sensor_analysis.filter_params_aggregated[filter_param_aggregated] = {}