Repository: pvlib/pvlib-python (python)

Issue:
Expose first_solar_spectral_correction in ModelChain and PVSystem
I suggest the following steps to expose the ``first_solar_spectral_correction`` function to ``ModelChain``:

1. Add a ``PVSystem.first_solar_spectral_correction(pw, airmass_absolute)`` method that wraps the ``atmosphere.first_solar_spectral_correction(pw, airmass_absolute, module_type=None, coefficients=None)`` function. This is straightforward except for deciding how to handle the keyword arguments. The Sandia module database has a ``Material`` key  and the CEC database has a ``Technology`` key. These keys could be mapped to the allowed ``module_type`` arguments. I'm not sure what to do about the ``coefficients`` option. Perhaps direct users to put their custom coefficients in ``module_parameters['fs_spectral_coefficients']``?

2. Implement the stub ``ModelChain.first_solar_spectral_loss`` method. Maybe something like:
```python
    def first_solar_spectral_loss(self):
        self.spectral_modifier = self.system.first_solar_spectral_loss(
            self.weather['precipitable_water'], self.airmass['airmass_absolute'])
        return self
``` 
I'm guessing that most users don't have PW in their weather dataframes, so this line is going to be a frequent source of errors. We might consider adding a ``try/except KeyError`` with a more helpful error message.

3. Improve ``ModelChain.infer_spectral_model``. This probably amounts to setting ``mc._spectral_model = mc.first_solar_spectral_loss`` if a CEC module or a ``module_parameters['fs_spectral_coefficients']`` key is detected on the ``PVSystem`` object.

4. Optional. Add a dewpoint temperature to precipitable water conversion to the ``ModelChain.prepare_inputs`` method.

5. Add tests.

6. Update documentation.

It's unfortunate that ``sapm_spectral_loss`` and ``first_solar_spectral_correction`` live in different modules. Fixing that could be step 0, or we could leave it alone.

I'm in no rush to implement this, but I'd help anyone with a pull request.

see also #358

Pull request:
Expose first_solar_spectral_correction
pvlib python pull request guidelines
====================================

Thank you for your contribution to pvlib python!

You may submit a pull request with your code at any stage of completion, however, before the code can be merged the following items must be addressed:

 - [x] Closes #359
 - [x] Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests must pass on the TravisCI and Appveyor testing services.
 - [x] Code quality and style is sufficient. Passes ``git diff upstream/master -u -- "*.py" | flake8 --diff`` and/or landscape.io linting service.
 - [x] New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
 - [x] Updates entries to `docs/sphinx/source/api.rst` for API changes.
 - [x] Adds description and name entries in the appropriate `docs/sphinx/source/whatsnew` file for all changes.

Please don't hesitate to ask for help if you're unsure of how to accomplish any of the above. You may delete all of these instructions except for the list above.

Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

#359 complete

Patch:
diff --git a/docs/sphinx/source/whatsnew/v0.6.0.rst b/docs/sphinx/source/whatsnew/v0.6.0.rst
index 0eed2c2..5ca7f34 100644
--- a/docs/sphinx/source/whatsnew/v0.6.0.rst
+++ b/docs/sphinx/source/whatsnew/v0.6.0.rst
@@ -11,10 +11,14 @@ API Changes
 Enhancements
 ~~~~~~~~~~~~
 * Add sea surface albedo in irradiance.py (:issue:`458`)
+* Implement first_solar_spectral_loss in modelchain.py (:issue:'359')
+
 
 Bug fixes
 ~~~~~~~~~
 * Unset executable bits of irradiance.py and test_irradiance.py (:issue:`460`)
+* Fix failing tests due to column order on Python 3.6+ and Pandas 0.23+
+  (:issue:`464`)
 
 
 Documentation
@@ -29,3 +33,5 @@ Contributors
 ~~~~~~~~~~~~
 * Will Holmgren
 * Yu Cao
+* Cliff Hansen
+
diff --git a/pvlib/atmosphere.py b/pvlib/atmosphere.py
index 4878cc7..ee57135 100644
--- a/pvlib/atmosphere.py
+++ b/pvlib/atmosphere.py
@@ -466,8 +466,11 @@ def first_solar_spectral_correction(pw, airmass_absolute, module_type=None,
         coefficients = _coefficients[module_type.lower()]
     elif module_type is None and coefficients is not None:
         pass
+    elif module_type is None and coefficients is None:
+        raise TypeError('No valid input provided, both module_type and ' +
+                        'coefficients are None')
     else:
-        raise TypeError('ambiguous input, must supply only 1 of ' +
+        raise TypeError('Cannot resolve input, must supply only one of ' +
                         'module_type and coefficients')
 
     # Evaluate Spectral Shift
diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py
index 8115939..29f4cfa 100644
--- a/pvlib/modelchain.py
+++ b/pvlib/modelchain.py
@@ -273,8 +273,8 @@ class ModelChain(object):
     spectral_model: None, str, or function, default None
         If None, the model will be inferred from the contents of
         system.module_parameters. Valid strings are 'sapm',
-        'first_solar' (not implemented), 'no_loss'. The ModelChain
-        instance will be passed as the first argument to a user-defined
+        'first_solar', 'no_loss'. The ModelChain instance will be passed 
+        as the first argument to a user-defined
         function.
 
     temp_model: str or function, default 'sapm'
@@ -518,7 +518,7 @@ class ModelChain(object):
         return self
 
     def no_aoi_loss(self):
-        self.aoi_modifier = 1
+        self.aoi_modifier = 1.0
         return self
 
     @property
@@ -532,7 +532,7 @@ class ModelChain(object):
         elif isinstance(model, str):
             model = model.lower()
             if model == 'first_solar':
-                raise NotImplementedError
+                self._spectral_model = self.first_solar_spectral_loss
             elif model == 'sapm':
                 self._spectral_model = self.sapm_spectral_loss
             elif model == 'no_loss':
@@ -546,12 +546,23 @@ class ModelChain(object):
         params = set(self.system.module_parameters.keys())
         if set(['A4', 'A3', 'A2', 'A1', 'A0']) <= params:
             return self.sapm_spectral_loss
+        elif ((('Technology' in params or
+                'Material' in params) and
+               (pvsystem._infer_cell_type() is not None)) or
+              'first_solar_spectral_coefficients' in params):
+            return self.first_solar_spectral_loss
         else:
             raise ValueError('could not infer spectral model from '
-                             'system.module_parameters')
+                             'system.module_parameters. Check that the '
+                             'parameters contain valid '
+                             'first_solar_spectral_coefficients or a valid '
+                             'Material or Technology value')
 
     def first_solar_spectral_loss(self):
-        raise NotImplementedError
+        self.spectral_modifier = self.system.first_solar_spectral_loss(
+                                        self.weather['precipitable_water'],
+                                        self.airmass['airmass_absolute'])
+        return self
 
     def sapm_spectral_loss(self):
         self.spectral_modifier = self.system.sapm_spectral_loss(
diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py
index 465b117..bcf1474 100644
--- a/pvlib/pvsystem.py
+++ b/pvlib/pvsystem.py
@@ -421,6 +421,94 @@ class PVSystem(object):
             poa_direct, poa_diffuse, airmass_absolute, aoi,
             self.module_parameters, reference_irradiance=reference_irradiance)
 
+    def first_solar_spectral_loss(self, pw, airmass_absolute):
+
+        """
+        Use the :py:func:`first_solar_spectral_correction` function to
+        calculate the spectral loss modifier. The model coefficients are
+        specific to the module's cell type, and are determined by searching
+        for one of the following keys in self.module_parameters (in order):
+            'first_solar_spectral_coefficients' (user-supplied coefficients)
+            'Technology' - a string describing the cell type, can be read from
+            the CEC module parameter database
+            'Material' - a string describing the cell type, can be read from
+            the Sandia module database.
+
+        Parameters
+        ----------
+        pw : array-like
+            atmospheric precipitable water (cm).
+
+        airmass_absolute : array-like
+            absolute (pressure corrected) airmass.
+
+        Returns
+        -------
+        modifier: array-like
+            spectral mismatch factor (unitless) which can be multiplied
+            with broadband irradiance reaching a module's cells to estimate
+            effective irradiance, i.e., the irradiance that is converted to
+            electrical current.
+        """
+
+        if 'first_solar_spectral_coefficients' in \
+                               self.module_parameters.keys():
+            coefficients = \
+                   self.module_parameters['first_solar_spectral_coefficients']
+            module_type = None
+        else:
+            module_type = self._infer_cell_type()
+            coefficients = None
+
+        return atmosphere.first_solar_spectral_correction(pw,
+                                                          airmass_absolute, 
+                                                          module_type,
+                                                          coefficients)
+
+    def _infer_cell_type(self):
+
+        """
+        Examines module_parameters and maps the Technology key for the CEC
+        database and the Material key for the Sandia database to a common
+        list of strings for cell type.
+
+        Returns
+        -------
+        cell_type: str
+
+        """
+
+        _cell_type_dict = {'Multi-c-Si': 'multisi',
+                           'Mono-c-Si': 'monosi',
+                           'Thin Film': 'cigs',
+                           'a-Si/nc': 'asi',
+                           'CIS': 'cigs',
+                           'CIGS': 'cigs',
+                           '1-a-Si': 'asi',
+                           'CdTe': 'cdte',
+                           'a-Si': 'asi',
+                           '2-a-Si': None,
+                           '3-a-Si': None,
+                           'HIT-Si': 'monosi',
+                           'mc-Si': 'multisi',
+                           'c-Si': 'multisi',
+                           'Si-Film': 'asi',
+                           'CdTe': 'cdte',
+                           'EFG mc-Si': 'multisi',
+                           'GaAs': None,
+                           'a-Si / mono-Si': 'monosi'}
+
+        if 'Technology' in self.module_parameters.keys():
+            # CEC module parameter set
+            cell_type = _cell_type_dict[self.module_parameters['Technology']]
+        elif 'Material' in self.module_parameters.keys():
+            # Sandia module parameter set
+            cell_type = _cell_type_dict[self.module_parameters['Material']]
+        else:
+            cell_type = None
+
+        return cell_type
+
     def singlediode(self, photocurrent, saturation_current,
                     resistance_series, resistance_shunt, nNsVth,
                     ivcurve_pnts=None):
diff --git a/pvlib/tracking.py b/pvlib/tracking.py
index ebbc35f..0b38eec 100644
--- a/pvlib/tracking.py
+++ b/pvlib/tracking.py
@@ -545,7 +545,8 @@ def singleaxis(apparent_zenith, apparent_azimuth,
                            'surface_azimuth': surface_azimuth,
                            'surface_tilt': surface_tilt},
                           index=times)
-
+    df_out = df_out[['tracker_theta', 'aoi',
+                     'surface_azimuth', 'surface_tilt']]
     df_out[apparent_zenith > 90] = np.nan
 
     return df_out


Test patch:
diff --git a/pvlib/test/test_modelchain.py b/pvlib/test/test_modelchain.py
index 9c13e4b..405db3e 100644
--- a/pvlib/test/test_modelchain.py
+++ b/pvlib/test/test_modelchain.py
@@ -173,12 +173,13 @@ def test_run_model_tracker(system, location):
                          index=times)
     assert_series_equal(ac, expected, check_less_precise=2)
 
-    expected = pd.DataFrame(np.
+    expect = pd.DataFrame(np.
         array([[ 54.82513187,  90.        ,  11.0039221 ,  11.0039221 ],
                [         nan,   0.        ,   0.        ,          nan]]),
         columns=['aoi', 'surface_azimuth', 'surface_tilt', 'tracker_theta'],
         index=times)
-    assert_frame_equal(mc.tracking, expected, check_less_precise=2)
+    expect = expect[['tracker_theta', 'aoi', 'surface_azimuth', 'surface_tilt']]
+    assert_frame_equal(mc.tracking, expect, check_less_precise=2)
 
 
 def poadc(mc):
@@ -262,28 +263,28 @@ def test_aoi_models(system, location, aoi_model, expected):
 def constant_spectral_loss(mc):
     mc.spectral_modifier = 0.9
 
+
 @requires_scipy
-@pytest.mark.parametrize('spectral_model, expected', [
-    ('sapm', [182.338436597, -2.00000000e-02]),
-    pytest.mark.xfail(raises=NotImplementedError)
-    (('first_solar', [179.371460714, -2.00000000e-02])),
-    ('no_loss', [181.604438144, -2.00000000e-02]),
-    (constant_spectral_loss, [163.061569511, -2e-2])
+@pytest.mark.parametrize('spectral_model', [
+        'sapm', 'first_solar', 'no_loss', constant_spectral_loss
 ])
-def test_spectral_models(system, location, spectral_model, expected):
+def test_spectral_models(system, location, spectral_model):
+    times = pd.date_range('20160101 1200-0700', periods=3, freq='6H')
+    weather = pd.DataFrame(data=[0.3, 0.5, 1.0],
+                           index=times,
+                           columns=['precipitable_water'])
     mc = ModelChain(system, location, dc_model='sapm',
                     aoi_model='no_loss', spectral_model=spectral_model)
-    times = pd.date_range('20160101 1200-0700', periods=2, freq='6H')
-    ac = mc.run_model(times).ac
-
-    expected = pd.Series(np.array(expected), index=times)
-    assert_series_equal(ac, expected, check_less_precise=2)
+    spectral_modifier = mc.run_model(times=times, 
+                                     weather=weather).spectral_modifier
+    assert isinstance(spectral_modifier, (pd.Series, float, int))
 
 
 def constant_losses(mc):
     mc.losses = 0.9
     mc.ac *= mc.losses
 
+
 @requires_scipy
 @pytest.mark.parametrize('losses_model, expected', [
     ('pvwatts', [163.280464174, 0]),
diff --git a/pvlib/test/test_pvsystem.py b/pvlib/test/test_pvsystem.py
index 42c696d..7ea9fde 100644
--- a/pvlib/test/test_pvsystem.py
+++ b/pvlib/test/test_pvsystem.py
@@ -266,6 +266,16 @@ def test_PVSystem_sapm_spectral_loss(sapm_module_params):
     out = system.sapm_spectral_loss(airmass)
 
 
+@pytest.mark.parametrize("expected", [1.03173953])
+def test_PVSystem_first_solar_spectral_loss(sapm_module_params, expected):
+    system = pvsystem.PVSystem(module_parameters=sapm_module_params)
+    pw = 3
+    airmass_absolute = 3
+    out = system.first_solar_spectral_loss(pw, airmass_absolute)
+
+    assert_allclose(out, expected, atol=1e-4)
+
+
 @pytest.mark.parametrize('aoi,expected', [
     (45, 0.9975036250000002),
     (np.array([[-30, 30, 100, np.nan]]),
diff --git a/pvlib/test/test_tracking.py b/pvlib/test/test_tracking.py
index a1ac8cb..c8c6c09 100644
--- a/pvlib/test/test_tracking.py
+++ b/pvlib/test/test_tracking.py
@@ -11,6 +11,8 @@ from numpy.testing import assert_allclose
 from pvlib.location import Location
 from pvlib import tracking
 
+SINGLEAXIS_COL_ORDER = ['tracker_theta', 'aoi',
+                        'surface_azimuth', 'surface_tilt']
 
 def test_solar_noon():
     apparent_zenith = pd.Series([10])
@@ -20,9 +22,10 @@ def test_solar_noon():
                                        max_angle=90, backtrack=True,
                                        gcr=2.0/7.0)
 
-    expect = pd.DataFrame({'aoi': 10, 'surface_azimuth': 90,
-                           'surface_tilt': 0, 'tracker_theta': 0},
+    expect = pd.DataFrame({'tracker_theta': 0, 'aoi': 10,
+                           'surface_azimuth': 90, 'surface_tilt': 0},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -36,9 +39,10 @@ def test_azimuth_north_south():
                                        max_angle=90, backtrack=True,
                                        gcr=2.0/7.0)
 
-    expect = pd.DataFrame({'aoi': 0, 'surface_azimuth': 90,
-                           'surface_tilt': 60, 'tracker_theta': -60},
+    expect = pd.DataFrame({'tracker_theta': -60, 'aoi': 0,
+                           'surface_azimuth': 90, 'surface_tilt': 60},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -63,6 +67,7 @@ def test_max_angle():
     expect = pd.DataFrame({'aoi': 15, 'surface_azimuth': 90,
                            'surface_tilt': 45, 'tracker_theta': 45},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -79,6 +84,7 @@ def test_backtrack():
     expect = pd.DataFrame({'aoi': 0, 'surface_azimuth': 90,
                            'surface_tilt': 80, 'tracker_theta': 80},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -90,6 +96,7 @@ def test_backtrack():
     expect = pd.DataFrame({'aoi': 52.5716, 'surface_azimuth': 90,
                            'surface_tilt': 27.42833, 'tracker_theta': 27.4283},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -104,8 +111,10 @@ def test_axis_tilt():
                                        gcr=2.0/7.0)
 
     expect = pd.DataFrame({'aoi': 7.286245, 'surface_azimuth': 142.65730,
-                           'surface_tilt': 35.98741, 'tracker_theta': -20.88121},
+                           'surface_tilt': 35.98741,
+                           'tracker_theta': -20.88121},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -117,6 +126,7 @@ def test_axis_tilt():
     expect = pd.DataFrame({'aoi': 47.6632, 'surface_azimuth': 50.96969,
                            'surface_tilt': 42.5152, 'tracker_theta': 31.6655},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -133,6 +143,7 @@ def test_axis_azimuth():
     expect = pd.DataFrame({'aoi': 30, 'surface_azimuth': 180,
                            'surface_tilt': 0, 'tracker_theta': 0},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -147,6 +158,7 @@ def test_axis_azimuth():
     expect = pd.DataFrame({'aoi': 0, 'surface_azimuth': 180,
                            'surface_tilt': 30, 'tracker_theta': 30},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -184,8 +196,10 @@ def test_SingleAxisTracker_tracking():
     tracker_data = system.singleaxis(apparent_zenith, apparent_azimuth)
 
     expect = pd.DataFrame({'aoi': 7.286245, 'surface_azimuth': 142.65730 ,
-                           'surface_tilt': 35.98741, 'tracker_theta': -20.88121},
+                           'surface_tilt': 35.98741,
+                           'tracker_theta': -20.88121},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)
 
@@ -202,9 +216,11 @@ def test_SingleAxisTracker_tracking():
     apparent_azimuth = pd.Series([180+pvsyst_solar_azimuth])
     apparent_zenith = pd.Series([90-pvsyst_solar_height])
     tracker_data = pvsyst_system.singleaxis(apparent_zenith, apparent_azimuth)
-    expect = pd.DataFrame({'aoi': 41.07852 , 'surface_azimuth': 180-18.432 ,
-                           'surface_tilt': 24.92122 , 'tracker_theta': -15.18391},
+    expect = pd.DataFrame({'aoi': 41.07852 , 'surface_azimuth': 180-18.432,
+                           'surface_tilt': 24.92122 ,
+                           'tracker_theta': -15.18391},
                            index=[0], dtype=np.float64)
+    expect = expect[SINGLEAXIS_COL_ORDER]
 
     assert_frame_equal(expect, tracker_data)