Repository: pvlib/pvlib-python
Pull request #1052 (2020-09-07 02:49:32)

fix modelchain poa tests
 - [ ] Closes #xxxx
 - [x] I am familiar with the [contributing guidelines](https://pvlib-python.readthedocs.io/en/latest/contributing.html)
 - [x] Tests added
 - [x] Updates entries to [`docs/sphinx/source/api.rst`](https://github.com/pvlib/pvlib-python/blob/master/docs/sphinx/source/api.rst) for API changes.
 - [x] Adds description and name entries in the appropriate "what's new" file in [`docs/sphinx/source/whatsnew`](https://github.com/pvlib/pvlib-python/tree/master/docs/sphinx/source/whatsnew) for all changes. Includes link to the GitHub Issue with `` :issue:`num` `` or this Pull Request with `` :pull:`num` ``. Includes contributor name and/or GitHub username (link with `` :ghuser:`user` ``).
 - [x] New code is fully documented. Includes [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html) compliant docstrings, examples, and comments where necessary.
 - [x] Pull request is nearly complete and ready for detailed review.
 - [x] Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

As discussed [here](https://github.com/pvlib/pvlib-python/pull/943/files#r484108743), `test_prepare_inputs_from_poa` didn't work when I ran the tests locally (as I expect from looking at the code) but the CI never failed (for reasons I still don't understand). This PR fixes that.

This PR also changes the `modelchain.WEATHER_KEYS`, `POA_DATA_KEYS`, `TEMPERATURE_KEYS`, and `DATA_KEYS` to tuples. The problem with using a set here is that it's unordered, so the `_assign_weather` and `_assign_total_irrad` create dataframes with column order that depends on the platform. 

wholmgren: While we're at it, I wonder about the name `POA_DATA_KEYS`. Would `POA_KEYS` be more consistent with `WEATHER_KEYS` and `TEMPERATURE_KEYS`?
wholmgren: I was not able to figure that out, unfortunately. The fixtures are all function scoped either explicitly or by default, so I don't think they're the cause of the problem. 

Patch:
diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py
index c9ba17a..e095680 100644
--- a/pvlib/modelchain.py
+++ b/pvlib/modelchain.py
@@ -21,20 +21,20 @@ from pvlib.tools import _build_kwargs
 # keys that are used to detect input data and assign data to appropriate
 # ModelChain attribute
 # for ModelChain.weather
-WEATHER_KEYS = {'ghi', 'dhi', 'dni', 'wind_speed', 'temp_air',
-                'precipitable_water'}
+WEATHER_KEYS = ('ghi', 'dhi', 'dni', 'wind_speed', 'temp_air',
+                'precipitable_water')
 
 # for ModelChain.total_irrad
-POA_DATA_KEYS = {'poa_global', 'poa_direct', 'poa_diffuse'}
+POA_KEYS = ('poa_global', 'poa_direct', 'poa_diffuse')
 
 # Optional keys to communicate temperature data. If provided,
 # 'cell_temperature' overrides ModelChain.temperature_model and sets
 # ModelChain.cell_temperature to the data. If 'module_temperature' is provdied,
 # overrides ModelChain.temperature_model with
 # pvlib.temperature.sapm_celL_from_module
-TEMPERATURE_KEYS = {'module_temperature', 'cell_temperature'}
+TEMPERATURE_KEYS = ('module_temperature', 'cell_temperature')
 
-DATA_KEYS = WEATHER_KEYS | POA_DATA_KEYS | TEMPERATURE_KEYS
+DATA_KEYS = WEATHER_KEYS + POA_KEYS + TEMPERATURE_KEYS
 
 # these dictionaries contain the default configuration for following
 # established modeling sequences. They can be used in combination with
@@ -1090,7 +1090,7 @@ class ModelChain:
         return self
 
     def _assign_total_irrad(self, data):
-        key_list = [k for k in POA_DATA_KEYS if k in data]
+        key_list = [k for k in POA_KEYS if k in data]
         self.total_irrad = data[key_list].copy()
         return self
 


Test patch:
diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py
index 494e402..1758b0c 100644
--- a/pvlib/tests/test_modelchain.py
+++ b/pvlib/tests/test_modelchain.py
@@ -181,8 +181,8 @@ def weather():
 @pytest.fixture
 def total_irrad(weather):
     return pd.DataFrame({'poa_global': [800., 500.],
-                         'poa_diffuse': [300., 200.],
-                         'poa_direct': [500., 300.]}, index=weather.index)
+                         'poa_direct': [500., 300.],
+                         'poa_diffuse': [300., 200.]}, index=weather.index)
 
 
 def test_ModelChain_creation(sapm_dc_snl_ac_system, location):
@@ -336,21 +336,25 @@ def test_run_model_tracker(sapm_dc_snl_ac_system, location, weather, mocker):
 
 def test__assign_total_irrad(sapm_dc_snl_ac_system, location, weather,
                              total_irrad):
-    weather[['poa_global', 'poa_diffuse', 'poa_direct']] = total_irrad
+    data = pd.concat([weather, total_irrad], axis=1)
     mc = ModelChain(sapm_dc_snl_ac_system, location)
-    mc._assign_total_irrad(weather)
-    for k in modelchain.POA_DATA_KEYS:
-        assert_series_equal(mc.total_irrad[k], total_irrad[k])
+    mc._assign_total_irrad(data)
+    assert_frame_equal(mc.total_irrad, total_irrad)
 
 
 def test_prepare_inputs_from_poa(sapm_dc_snl_ac_system, location,
                                  weather, total_irrad):
-    data = weather.copy()
-    data[['poa_global', 'poa_diffuse', 'poa_direct']] = total_irrad
+    data = pd.concat([weather, total_irrad], axis=1)
     mc = ModelChain(sapm_dc_snl_ac_system, location)
     mc.prepare_inputs_from_poa(data)
+    weather_expected = weather.copy()
+    weather_expected['temp_air'] = 20
+    weather_expected['wind_speed'] = 0
+    # order as expected
+    weather_expected = weather_expected[
+        ['ghi', 'dhi', 'dni', 'wind_speed', 'temp_air']]
     # weather attribute
-    assert_frame_equal(mc.weather, weather)
+    assert_frame_equal(mc.weather, weather_expected)
     # total_irrad attribute
     assert_frame_equal(mc.total_irrad, total_irrad)