Merge branch 'fix_issue_#370' of https://github.com/NREL/rdtools into fix_issue_#370

Author: martin-springer

CITATION.cff

@@ -0,0 +1,46 @@
+cff-version: 1.2.0
+message: "If you use this software, please cite it as below."
+authors:
+- family-names: "Deceglie"
+  given-names: "Michael G."
+  orcid: "https://orcid.org/0000-0001-7063-9676"
+- family-names: "Anderson"
+  given-names: "Kevin"
+- family-names: "Shinn"
+  given-names: "Adam"
+- family-names: "Ambarish"
+  given-names: "Nag"
+- family-names: "Mikofski"
+  given-names: "Mark"
+  orcid: "https://orcid.org/0000-0001-8001-8582"
+- family-names: "Springer"
+  given-names: "Martin"
+  orcid: "https://orcid.org/0000-0001-6803-108X"
+- family-names: "Yan"
+  given-names: "Jiyang"
+- family-names: "Perry"
+  given-names: "Kirsten"
+- family-names: "Villamar"
+  given-names: "Sandra"
+- family-names: "Vining"
+  given-names: "Will"
+- family-names: "Kimball"
+  given-names: "Gregory M."
+  orcid: "https://orcid.org/0000-0003-1075-1417"
+- family-names: "Ruth"
+  given-names: "Daniel"
+- family-names: "Moyer"
+  given-names: "Noah"
+- family-names: "Nguyen"
+  given-names: "Quyen"
+- family-names: "Jordan"
+  given-names: "Dirk"
+  orcid: "https://orcid.org/0000-0002-2183-7489"
+- family-names: "Muller"
+  given-names: "Matthew"
+- family-names: "Deline"
+  given-names: "Chris"
+  orcid: "https://orcid.org/0000-0002-9867-8930"
+title: "RdTools"
+doi: 10.5281/zenodo.1210316
+url: "https://github.com/NREL/rdtools"

README.md

@@ -13,7 +13,7 @@ RdTools is an open-source library to support reproducible technical analysis of
 time series data from photovoltaic energy systems. The library aims to provide
 best practice analysis routines along with the building blocks for users to
 tailor their own analyses. Current applications include the evaluation of PV
-production over several years to obtain rates of performance degradation and 
+production over several years to obtain rates of performance degradation and
 soiling loss. RdTools can handle both high frequency (hourly or better) or low
 frequency (daily, weekly, etc.) datasets. Best results are obtained with higher
 frequency data.
@@ -34,10 +34,10 @@ RdTools currently is tested on Python 3.9+.
 To cite RdTools, please use the following along with the version number
 and the specific DOI coresponding to that version from [Zenodo](https://doi.org/10.5281/zenodo.1210316):
 
--  Michael G. Deceglie, Ambarish Nag, Adam Shinn, Gregory Kimball,
-   Daniel Ruth, Dirk Jordan, Jiyang Yan, Kevin Anderson, Kirsten Perry,
-   Mark Mikofski, Matthew Muller, Will Vining, and Chris Deline,
-   RdTools, version {insert version}, Computer Software,
+-  Michael G. Deceglie, Kevin Anderson,  Adam Shinn, Ambarish Nag, Mark Mikofski,
+   Martin Springer, Jiyang Yan, Kirsten Perry, Sandra Villamar, Will Vining,
+   Gregory Kimball, Daniel Ruth, Noah Moyer, Quyen Nguyen, Dirk Jordan,
+   Matthew Muller, and Chris Deline, RdTools, version {insert version}, Computer Software,
    https://github.com/NREL/rdtools. DOI:{insert DOI}
 
 The underlying workflow of RdTools has been published in several places.

docs/TrendAnalysis_example.ipynb

@@ -12,4 +12,3 @@ Bug fixes
 ---------
 * Set marker linewidth to zero in `rdtools.plotting.degradation_summary_plots` (:pull:`433`)
 * Fix :py:func:`~rdtools.normalization.energy_from_power` returns incorrect index for shifted hourly data (:issue:`370`, :pull:`437`)
-

docs/TrendAnalysis_example_NSRDB.ipynb

@@ -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))

docs/degradation_and_soiling_example.ipynb

@@ -112,8 +112,9 @@ def degradation_summary_plots(yoy_rd, yoy_ci, yoy_info, normalized_yield,
         colors = yoy_info['usage_of_points'].map({0: 'red', 1: 'green', 2: plot_color})
     else:
         colors = plot_color
-    ax1.scatter(renormalized_yield.index, renormalized_yield,
-                c=colors, alpha=scatter_alpha)
+    ax1.scatter(
+        renormalized_yield.index, renormalized_yield, c=colors, alpha=scatter_alpha, linewidths=0
+    )
 
     ax1.plot(x, y, 'k--', linewidth=3)
     ax1.set_xlabel('Date')

docs/sphinx/source/changelog/pending.rst

@@ -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
@@ -758,3 +759,92 @@ def test_energy_from_power_shifted_hourly_data():
 
     energy = normalization.energy_from_power(pv)
     pd.testing.assert_series_equal(energy, pv[1:], check_names=False)
+
+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] = {}
+