Repository: pvlib/pvanalytics

Issue:
Handling missing data in outliers
Hello, first of all very interesting project!

I was trying out the outlier funcions. The `outliers.zscore` function (which depends on `scipy.stats.zscore`) returns a series full of NaNs when at least one NaN is present in data.

Would it be helpful to use the option of ignoring such data? Like `stats.zscore(data, nan_policy='omit')` or dropping NaN values?
Or handling this type of data is not to be contemplated?

Discussion:
wfvining: @MichaelHopwood I enabled the CI pipeline for you, sorry about the delay, I think this is a new feature of Github Actions.
MichaelHopwood: Thanks @wfvining! I believe I fixed the minor linting issues but will need to confirm once the CI pipeline is activated again.
wfvining: @MichaelHopwood sorry about that... this is a very inconvenient "feature." I'll keep a close eye to try to approve runs quickly. Apparently people were using actions for crypto mining, so there isn't a way to just grant blanked approval.

I would recommend running `pytest pvanalytics` and `flake8` locally to speed up the feedback loop for yourself for the time being.

Once this PR is merged any future contributions should not require maintainer approval for the actions pipeline to run.
MichaelHopwood: Hi @wfvining, `scipy.stats.zscore` was given the parameter `nan_policy` in release 1.4.0 [1]. For this PR to run successfully, the requirements files would have to reflect the newer version of scipy. I'll leave it up to you to decide. 👍 

[1] http://scipy.github.io/devdocs/release.1.4.0.html#scipy-stats-improvements
wfvining: Shoot, I did not realize that parameter was added so recently. We've been trying to apply the NEP-29 rules for dependencies, so that means we should not require scipy > 1.2.X yet. 1.4 isn't that new, but still withing 24 months (December, 2019) and 3 minor versions.

It would not be too difficult to mimic the scipy `raise` behavior, the other options look somewhat more complex, and I'm not sure we want to got to the effort of recreating them. Maybe just `raise` is enough, with a useful message that indicates users should get rid of their nans. What do you think?
wfvining: Another option would be to try passing `nan_policy` and catch the `TypeError` if the older version of scipy is installed. In that case, we would revert to raising our own error if there are nans, or calling `zscore` without the `nan_policy` arg if there are not. We'd probably need to also raise a `ValueError` if the user passes something other than `'raise'` in this case, something that says to upgrade to scipy >= 1.4 if they need that behavior. 
cwhanse: If we're not advancing `scipy` then I would favor mimicking the `raise` and `omit` behavior. I don't think that `propagate` makes much sense here (any `nan` in the input makes the output `nan`). If `raise`, any `nan` in input raises a `ValueError`; if `omit`, the zscore  is returned in position where data is not `nan`, along with `nan` in positions corresponding to input.
wfvining: `propagate` is the behavior prior to 1.4, so we kind of get that for free. I looked at how scipy handles `omit` and it is a bit more involved than I expected. Maybe for our purposes it wouldn't be so complex: Just drop the nans, compute the zscores, reindex and restore the nans?
cwhanse: > Just drop the nans, compute the zscores, reindex and restore the nans?

That's what I am thinking.

Patch:
diff --git a/docs/whatsnew/0.1.1.rst b/docs/whatsnew/0.1.1.rst
index e994726..a44d1e1 100644
--- a/docs/whatsnew/0.1.1.rst
+++ b/docs/whatsnew/0.1.1.rst
@@ -10,13 +10,18 @@ Enhancements
 ~~~~~~~~~~~~
 
 * Quantification of irradiance variability with
-  :py:func:`pvanalytics.metrics.variability_index`.
-
+  :py:func:`pvanalytics.metrics.variability_index`. (:issue:`60`, :pull:`106`)
+* Internal refactor of `pvanalytics.metrics.performance_ratio_nrel` to support
+  other performance ratio formulas. (:pull:`109`)
 
 Bug Fixes
 ~~~~~~~~~
 
+* Added nan_policy to zscore calculation
+  :py:func:`pvanalytics.quality.outliers.zscore`.
+
 Contributors
 ~~~~~~~~~~~~
 
 * Kevin Anderson (:ghuser:`kanderso-nrel`)
+* Cliff Hansen (:ghuser:`cwhanse`)
diff --git a/pvanalytics/metrics.py b/pvanalytics/metrics.py
index bac36a2..c6b5ab0 100644
--- a/pvanalytics/metrics.py
+++ b/pvanalytics/metrics.py
@@ -4,6 +4,19 @@ from pvlib.temperature import sapm_cell
 from pvlib.pvsystem import pvwatts_dc
 
 
+def _performance_ratio(measured, modeled):
+    """ Returns the ratio sum(measured) / sum(modeled)
+    """
+    return measured.sum() / modeled.sum()
+
+
+def _calc_cell_temp_weighted(cell_temperature, irradiance):
+    """ Returns the ratio sum(cell_temperature * irradiance) / sum(irradiance)
+    """
+    numerator = cell_temperature * irradiance
+    return numerator.sum() / irradiance.sum()
+
+
 def performance_ratio_nrel(poa_global, temp_air, wind_speed, pac, pdc0,
                            a=-3.56, b=-0.075, deltaT=3, gamma_pdc=-0.00433):
     r"""
@@ -56,15 +69,12 @@ def performance_ratio_nrel(poa_global, temp_air, wind_speed, pac, pdc0,
     cell_temperature = sapm_cell(poa_global, temp_air, wind_speed, a, b,
                                  deltaT)
 
-    tcell_poa_global = poa_global * cell_temperature
-    tref = tcell_poa_global.sum() / poa_global.sum()
-
-    pdc = pvwatts_dc(poa_global, cell_temperature, pdc0, gamma_pdc,
-                     temp_ref=tref)
+    tavg = _calc_cell_temp_weighted(cell_temperature, poa_global)
 
-    performance_ratio = pac.sum() / pdc.sum()
+    modeled = pvwatts_dc(poa_global, cell_temperature, pdc0, gamma_pdc,
+                         temp_ref=tavg)
 
-    return performance_ratio
+    return _performance_ratio(pac, modeled)
 
 
 def _calc_pathlength(signal, freq):
diff --git a/pvanalytics/quality/outliers.py b/pvanalytics/quality/outliers.py
index 7bb0f62..e5d4dc5 100644
--- a/pvanalytics/quality/outliers.py
+++ b/pvanalytics/quality/outliers.py
@@ -38,7 +38,7 @@ def tukey(data, k=1.5):
             | (data > (third_quartile + k*iqr)))
 
 
-def zscore(data, zmax=1.5):
+def zscore(data, zmax=1.5, nan_policy='raise'):
     """Identify outliers using the z-score.
 
     Points with z-score greater than `zmax` are considered as outliers.
@@ -49,6 +49,11 @@ def zscore(data, zmax=1.5):
         A series of numeric values in which to find outliers.
     zmax : float
         Upper limit of the absolute values of the z-score.
+    nan_policy : str, default 'raise'
+        Define how to handle NaNs in the input series.
+        If 'raise', a ValueError is raised when `data` contains NaNs.
+        If 'omit', NaNs are ignored and False is returned at indices that
+        contained NaN in `data`.
 
     Returns
     -------
@@ -57,7 +62,25 @@ def zscore(data, zmax=1.5):
         outlier.
 
     """
-    return pd.Series((abs(stats.zscore(data)) > zmax), index=data.index)
+    data = data.copy()
+    nan_mask = pd.Series([False] * len(data))
+
+    if data.hasnans:
+        if nan_policy == 'raise':
+            raise ValueError("The input contains nan values.")
+        elif nan_policy == 'omit':
+            nan_mask = data.isna()
+        else:
+            raise ValueError(f"Unnexpected value ({nan_policy}) passed to "
+                             "nan_policy. Expected 'raise' or 'omit'.")
+
+    data[~nan_mask] = pd.Series(abs(stats.zscore(data[~nan_mask])) > zmax,
+                                index=data[~nan_mask].index)
+
+    # Place False where original series had NaNs
+    data[nan_mask] = False
+    # Return a boolean-casted series
+    return data.astype(bool)
 
 
 def hampel(data, window=5, max_deviation=3.0, scale=None):


Test patch:
diff --git a/pvanalytics/tests/quality/test_outliers.py b/pvanalytics/tests/quality/test_outliers.py
index 22e0cde..8e03691 100644
--- a/pvanalytics/tests/quality/test_outliers.py
+++ b/pvanalytics/tests/quality/test_outliers.py
@@ -1,4 +1,5 @@
 """Tests for the quality.outliers module."""
+import pytest
 import pandas as pd
 import numpy as np
 from pandas.util.testing import assert_series_equal
@@ -43,6 +44,28 @@ def test_tukey_lower_criteria():
     )
 
 
+def test_zscore_raise_nan_input():
+    data = pd.Series([1, 0, -1, 0, np.NaN, 1, -1, 10])
+
+    with pytest.raises(ValueError):
+        outliers.zscore(data, nan_policy='raise')
+
+
+def test_zscore_invalid_nan_policy():
+    data = pd.Series([1, 0, -1, 0, np.NaN, 1, -1, 10])
+
+    with pytest.raises(ValueError):
+        outliers.zscore(data, nan_policy='incorrect_str')
+
+
+def test_zscore_omit_nan_input():
+    data = pd.Series([1, 0, -1, 0, np.NaN, 1, -1, 10])
+    assert_series_equal(
+        pd.Series([False, False, False, False, False, False, False, True]),
+        outliers.zscore(outliers.zscore(data, nan_policy='omit'))
+    )
+
+
 def test_zscore_all_same():
     """If all data is identical there are no outliers."""
     data = pd.Series([1 for _ in range(20)])