code review

* doi backticks
* cell_type --> module_type
* spectrum.martin_ruiz_spectral_modifer --> spectrum.martin_ruiz
* change hastattr __iter__ to np.isscalar
* behaviour: providing module_type and model_parameters raises an error instead of a warning

Author: echedey-ls

docs/sphinx/source/reference/effects_on_pv_system_output.rst

@@ -52,4 +52,4 @@ Spectrum
    spectrum.get_example_spectral_response
    spectrum.get_am15g
    spectrum.calc_spectral_mismatch_field
-   spectrum.martin_ruiz_spectral_modifier
+   spectrum.martin_ruiz

pvlib/spectrum/__init__.py

@@ -1,4 +1,4 @@
 from pvlib.spectrum.spectrl2 import spectrl2  # noqa: F401
 from pvlib.spectrum.mismatch import (get_example_spectral_response, get_am15g,
                                      calc_spectral_mismatch_field,
-                                     martin_ruiz_spectral_modifier)
+                                     martin_ruiz)

pvlib/spectrum/mismatch.py

@@ -7,7 +7,6 @@
 import pandas as pd
 from scipy.interpolate import interp1d
 import os
-from warnings import warn
 
 
 def get_example_spectral_response(wavelength=None):
@@ -238,8 +237,8 @@ def integrate(e):
     return smm
 
 
-def martin_ruiz_spectral_modifier(clearness_index, airmass_absolute,
-                                  cell_type=None, model_parameters=None):
+def martin_ruiz(clearness_index, airmass_absolute, module_type=None,
+                model_parameters=None):
     r"""
     Calculate spectral mismatch modifiers for POA direct, sky diffuse and
     ground diffuse irradiances using the clearness index and the absolute
@@ -261,7 +260,7 @@ def martin_ruiz_spectral_modifier(clearness_index, airmass_absolute,
         recommended for default parameters of ``monosi``, ``polysi`` and
         ``asi``, see [1]_).
 
-    cell_type : string, optional
+    module_type : string, optional
         Specifies material of the cell in order to infer model parameters.
         Allowed types are ``monosi``, ``polysi`` and ``asi``, either lower or
         upper case. If not specified, ``model_parameters`` must be provided.
@@ -272,7 +271,7 @@ def martin_ruiz_spectral_modifier(clearness_index, airmass_absolute,
         .. code-block:: python
 
             # Using a dict
-            # Return keys are the same as specifying 'cell_type'
+            # Return keys are the same as specifying 'module_type'
             model_parameters = {
                 'poa_direct': {'c': c1, 'a': a1, 'b': b1},
                 'poa_sky_diffuse': {'c': c2, 'a': a2, 'b': b2},
@@ -305,9 +304,9 @@ def martin_ruiz_spectral_modifier(clearness_index, airmass_absolute,
     ValueError
         If ``model_parameters`` is not suitable. See examples given above.
     TypeError
-        If neither ``cell_type`` nor ``model_parameters`` are given.
+        If neither ``module_type`` nor ``model_parameters`` are given.
     NotImplementedError
-        If ``cell_type`` is not found in internal table of parameters.
+        If ``module_type`` is not found in internal table of parameters.
 
     Notes
     -----
@@ -322,7 +321,7 @@ def martin_ruiz_spectral_modifier(clearness_index, airmass_absolute,
     ----------
     .. [1] Martín, N. and Ruiz, J.M. (1999), A new method for the spectral
        characterisation of PV modules. Prog. Photovolt: Res. Appl., 7: 299-310.
-       :doi:10.1002/(SICI)1099-159X(199907/08)7:4<299::AID-PIP260>3.0.CO;2-0
+       :doi:`10.1002/(SICI)1099-159X(199907/08)7:4<299::AID-PIP260>3.0.CO;2-0`
 
     See Also
     --------
@@ -346,39 +345,39 @@ def martin_ruiz_spectral_modifier(clearness_index, airmass_absolute,
         ])
 
     # Argument validation and choose components and model parameters
-    if cell_type is not None and model_parameters is None:
+    if module_type is not None and model_parameters is None:
         # Infer parameters from cell material
-        cell_type_lower = cell_type.lower()
-        if cell_type_lower in MARTIN_RUIZ_PARAMS.index:
-            _params = MARTIN_RUIZ_PARAMS.loc[cell_type_lower]
+        module_type_lower = module_type.lower()
+        if module_type_lower in MARTIN_RUIZ_PARAMS.index:
+            _params = MARTIN_RUIZ_PARAMS.loc[module_type_lower]
         else:
             raise NotImplementedError('Cell type parameters not defined in '
                                       'algorithm. Allowed types are '
                                       f'{tuple(MARTIN_RUIZ_PARAMS.index)}')
-    elif cell_type is None and model_parameters is None:
-        raise TypeError('You must pass at least "cell_type" '
-                        'or "model_parameters" as arguments.')
-    elif model_parameters is not None:  # Use user-defined model parameters
+    elif model_parameters is not None and module_type is None:
+        # Use user-defined model parameters
         # Validate 'model_parameters' sub-dicts keys
         if any([{'a', 'b', 'c'} != set(model_parameters[component].keys())
                 for component in model_parameters.keys()]):
             raise ValueError("You must specify model parameters with keys "
                              "'a','b','c' for each irradiation component.")
-
         _params = model_parameters
-        if cell_type is not None:
-            warn('Both "cell_type" and "model_parameters" given. '
-                 'Using provided "model_parameters".')
+    elif module_type is None and model_parameters is None:
+        raise TypeError('You must pass at least "module_type" '
+                        'or "model_parameters" as arguments.')
+    elif model_parameters is not None and module_type is not None:
+        raise TypeError('Cannot resolve input: must supply only one of '
+                        '"module_type" or "model_parameters"')
+
+    if np.isscalar(clearness_index) and np.isscalar(airmass_absolute):
+        modifiers = dict(zip(IRRAD_COMPONENTS, (np.nan,)*3))
+    else:
+        modifiers = pd.DataFrame(columns=IRRAD_COMPONENTS)
 
     # Compute difference here to avoid recalculating inside loop
     kt_delta = clearness_index - 0.74
     am_delta = airmass_absolute - 1.5
 
-    if hasattr(kt_delta, '__iter__') or hasattr(am_delta, '__iter__'):
-        modifiers = pd.DataFrame(columns=IRRAD_COMPONENTS)
-    else:
-        modifiers = dict(zip(IRRAD_COMPONENTS, (np.nan,)*3))
-
     # Calculate mismatch modifier for each irradiation
     for irrad_type in IRRAD_COMPONENTS:
         # Skip irradiations not specified in 'model_params'

pvlib/tests/test_spectrum.py

@@ -176,7 +176,7 @@ def test_calc_spectral_mismatch_field(spectrl2_data):
 
 @pytest.fixture
 def martin_ruiz_mismatch_data():
-    # Data to run tests of martin_ruiz_spectral_modifier
+    # Data to run tests of spectrum.martin_ruiz
     kwargs = {
         'clearness_index': [0.56, 0.612, 0.664, 0.716, 0.768, 0.82],
         'airmass_absolute': [2, 1.8, 1.6, 1.4, 1.2, 1],
@@ -205,12 +205,12 @@ def martin_ruiz_mismatch_data():
 
 
 def test_martin_ruiz_mm_scalar(martin_ruiz_mismatch_data):
-    # test scalar input ; only cell_type given
+    # test scalar input ; only module_type given
     clearness_index = martin_ruiz_mismatch_data['clearness_index'][0]
     airmass_absolute = martin_ruiz_mismatch_data['airmass_absolute'][0]
-    result = spectrum.martin_ruiz_spectral_modifier(clearness_index,
-                                                    airmass_absolute,
-                                                    cell_type='asi')
+    result = spectrum.martin_ruiz(clearness_index,
+                                  airmass_absolute,
+                                  module_type='asi')
 
     assert_approx_equal(result['poa_direct'],
                         martin_ruiz_mismatch_data['asi_expected']['dir'][0],
@@ -224,31 +224,29 @@ def test_martin_ruiz_mm_scalar(martin_ruiz_mismatch_data):
 
 
 def test_martin_ruiz_mm_series(martin_ruiz_mismatch_data):
-    # test with Series input ; only cell_type given
+    # test with Series input ; only module_type given
     clearness_index = pd.Series(martin_ruiz_mismatch_data['clearness_index'])
     airmass_absolute = pd.Series(martin_ruiz_mismatch_data['airmass_absolute'])
     expected = pd.DataFrame(data={
         'dir': pd.Series(martin_ruiz_mismatch_data['polysi_expected']['dir']),
         'sky': pd.Series(martin_ruiz_mismatch_data['polysi_expected']['sky']),
         'gnd': pd.Series(martin_ruiz_mismatch_data['polysi_expected']['gnd'])})
 
-    result = spectrum.martin_ruiz_spectral_modifier(clearness_index,
-                                                    airmass_absolute,
-                                                    cell_type='polysi')
+    result = spectrum.martin_ruiz(clearness_index, airmass_absolute,
+                                  module_type='polysi')
     assert_allclose(result['poa_direct'], expected['dir'], atol=1e-5)
     assert_allclose(result['poa_sky_diffuse'], expected['sky'], atol=1e-5)
     assert_allclose(result['poa_ground_diffuse'], expected['gnd'], atol=1e-5)
 
 
 def test_martin_ruiz_mm_nans(martin_ruiz_mismatch_data):
-    # test NaN in, NaN out ; only cell_type given
+    # test NaN in, NaN out ; only module_type given
     clearness_index = pd.Series(martin_ruiz_mismatch_data['clearness_index'])
     airmass_absolute = pd.Series(martin_ruiz_mismatch_data['airmass_absolute'])
     airmass_absolute[:5] = np.nan
 
-    result = spectrum.martin_ruiz_spectral_modifier(clearness_index,
-                                                    airmass_absolute,
-                                                    cell_type='monosi')
+    result = spectrum.martin_ruiz(clearness_index, airmass_absolute,
+                                  module_type='monosi')
     assert np.isnan(result['poa_direct'][:5]).all()
     assert not np.isnan(result['poa_direct'][5:]).any()
     assert np.isnan(result['poa_sky_diffuse'][:5]).all()
@@ -268,7 +266,7 @@ def test_martin_ruiz_mm_model_dict(martin_ruiz_mismatch_data):
         'gnd': pd.Series(martin_ruiz_mismatch_data['monosi_expected']['gnd'])})
     model_parameters = martin_ruiz_mismatch_data['monosi_model_params_dict']
 
-    result = spectrum.martin_ruiz_spectral_modifier(
+    result = spectrum.martin_ruiz(
         clearness_index,
         airmass_absolute,
         model_parameters=model_parameters)
@@ -287,7 +285,7 @@ def test_martin_ruiz_mm_model_df(martin_ruiz_mismatch_data):
         'dir': np.array(martin_ruiz_mismatch_data['monosi_expected']['dir']),
         'sky': np.array(martin_ruiz_mismatch_data['monosi_expected']['sky'])})
 
-    result = spectrum.martin_ruiz_spectral_modifier(
+    result = spectrum.martin_ruiz(
         clearness_index,
         airmass_absolute,
         model_parameters=model_parameters)
@@ -296,44 +294,15 @@ def test_martin_ruiz_mm_model_df(martin_ruiz_mismatch_data):
     assert result['poa_ground_diffuse'].isna().all()
 
 
-def test_martin_ruiz_mm_userwarning(martin_ruiz_mismatch_data):
-    # test warning is raised with both 'cell_type' and 'model_parameters'
-    clearness_index = pd.Series(martin_ruiz_mismatch_data['clearness_index'])
-    airmass_absolute = pd.Series(martin_ruiz_mismatch_data['airmass_absolute'])
-    model_parameters = martin_ruiz_mismatch_data['monosi_model_params_dict']
-
-    with pytest.warns(UserWarning,
-                      match='Both "cell_type" and "model_parameters" given. '
-                            'Using provided "model_parameters".'):
-        _ = spectrum.martin_ruiz_spectral_modifier(
-            clearness_index,
-            airmass_absolute,
-            cell_type='asi',
-            model_parameters=model_parameters)
-
-
 def test_martin_ruiz_mm_error_notimplemented(martin_ruiz_mismatch_data):
-    # test exception is raised when cell_type does not exist in algorithm
+    # test exception is raised when module_type does not exist in algorithm
     clearness_index = np.array(martin_ruiz_mismatch_data['clearness_index'])
     airmass_absolute = np.array(martin_ruiz_mismatch_data['airmass_absolute'])
 
     with pytest.raises(NotImplementedError,
                        match='Cell type parameters not defined in algorithm.'):
-        _ = spectrum.martin_ruiz_spectral_modifier(clearness_index,
-                                                   airmass_absolute,
-                                                   cell_type='')
-
-
-def test_martin_ruiz_mm_error_missing_params(martin_ruiz_mismatch_data):
-    # test exception is raised when missing cell_type and model_parameters
-    clearness_index = np.array(martin_ruiz_mismatch_data['clearness_index'])
-    airmass_absolute = np.array(martin_ruiz_mismatch_data['airmass_absolute'])
-
-    with pytest.raises(TypeError,
-                       match='You must pass at least "cell_type" '
-                             'or "model_parameters" as arguments.'):
-        _ = spectrum.martin_ruiz_spectral_modifier(clearness_index,
-                                                   airmass_absolute)
+        _ = spectrum.martin_ruiz(clearness_index, airmass_absolute,
+                                 module_type='')
 
 
 def test_martin_ruiz_mm_error_model_keys(martin_ruiz_mismatch_data):
@@ -345,7 +314,30 @@ def test_martin_ruiz_mm_error_model_keys(martin_ruiz_mismatch_data):
     with pytest.raises(ValueError,
                        match="You must specify model parameters with keys "
                              "'a','b','c' for each irradiation component."):
-        _ = spectrum.martin_ruiz_spectral_modifier(
-            clearness_index,
-            airmass_absolute,
-            model_parameters=model_parameters)
+        _ = spectrum.martin_ruiz(clearness_index, airmass_absolute,
+                                 model_parameters=model_parameters)
+
+
+def test_martin_ruiz_mm_error_missing_params(martin_ruiz_mismatch_data):
+    # test exception is raised when missing module_type and model_parameters
+    clearness_index = np.array(martin_ruiz_mismatch_data['clearness_index'])
+    airmass_absolute = np.array(martin_ruiz_mismatch_data['airmass_absolute'])
+
+    with pytest.raises(TypeError,
+                       match='You must pass at least "module_type" '
+                             'or "model_parameters" as arguments.'):
+        _ = spectrum.martin_ruiz(clearness_index, airmass_absolute)
+
+
+def test_martin_ruiz_mm_error_too_many_arguments(martin_ruiz_mismatch_data):
+    # test warning is raised with both 'module_type' and 'model_parameters'
+    clearness_index = pd.Series(martin_ruiz_mismatch_data['clearness_index'])
+    airmass_absolute = pd.Series(martin_ruiz_mismatch_data['airmass_absolute'])
+    model_parameters = martin_ruiz_mismatch_data['monosi_model_params_dict']
+
+    with pytest.raises(TypeError,
+                       match='Cannot resolve input: must supply only one of '
+                             '"module_type" or "model_parameters"'):
+        _ = spectrum.martin_ruiz(clearness_index, airmass_absolute,
+                                 module_type='asi',
+                                 model_parameters=model_parameters)