Fix issue #352 and homogenize string concatenation across the code (#353)

* Fix bug in __repr__ method of ModelChain

* Homogenize the string concatenation and the __repr__ method across the code.

* Upate the test of ModelChain.__repr__ thanks to the code given by wholmgren in the comments discussing pull request #353

* Update the whatsnew/v0.4.6.rst file with the bug fix and the test update.

Author: Jonathan Gaffiot

docs/sphinx/source/whatsnew/v0.4.6.rst

@@ -10,6 +10,8 @@ Bug fixes
 * Method of multi-inheritance has changed to make it possible to use kwargs in
   the parent classes of LocalizedPVSystem and LocalizedSingleAxisTracker
   (:issue:`330`)
+* Fix the `__repr__` method of `ModelChain`, crashing when
+  `orientation_strategy` is set to `'None'` (:issue:`352`)
 
 
 Enhancements
@@ -28,6 +30,7 @@ Testing
 ~~~~~~~
 
 * Added explicit tests for aoi and aoi_projection functions.
+* Update test of `ModelChain.__repr__` to take in account :issue:`352`
 
 
 Contributors
@@ -37,3 +40,4 @@ Contributors
 * Uwe Krien
 * Alaina Kafkes
 * Birgit Schachler
+* Jonathan Gaffiot

pvlib/location.py

@@ -82,7 +82,7 @@ def __init__(self, latitude, longitude, tz='UTC', altitude=0,
     def __repr__(self):
         attrs = ['name', 'latitude', 'longitude', 'altitude', 'tz']
         return ('Location: \n  ' + '\n  '.join(
-            (attr + ': ' + str(getattr(self, attr)) for attr in attrs)))
+            ('{}: {}'.format(attr, getattr(self, attr)) for attr in attrs)))
 
     @classmethod
     def from_tmy(cls, tmy_metadata, tmy_data=None, **kwargs):
@@ -226,8 +226,8 @@ def get_clearsky(self, times, model='ineichen', solar_position=None,
                 apparent_elevation, pressure=pressure, dni_extra=dni_extra,
                 **kwargs)
         else:
-            raise ValueError(('{} is not a valid clear sky model. Must be ' +
-                              'one of ineichen, simplified_solis, haurwitz')
+            raise ValueError('{} is not a valid clear sky model. Must be '
+                             'one of ineichen, simplified_solis, haurwitz'
                              .format(model))
 
         return cs

pvlib/modelchain.py

@@ -112,7 +112,7 @@ def basic_chain(times, latitude, longitude,
         surface_tilt, surface_azimuth = \
             get_orientation(orientation_strategy, latitude=latitude)
     else:
-        raise ValueError('orientation_strategy or surface_tilt and ' +
+        raise ValueError('orientation_strategy or surface_tilt and '
                          'surface_azimuth must be provided')
 
     times = times
@@ -208,7 +208,7 @@ def get_orientation(strategy, **kwargs):
         surface_azimuth = 180
         surface_tilt = 0
     else:
-        raise ValueError('invalid orientation strategy. strategy must ' +
+        raise ValueError('invalid orientation strategy. strategy must '
                          'be one of south_at_latitude, flat,')
 
     return surface_tilt, surface_azimuth
@@ -344,7 +344,7 @@ def getmcattr(self, attr):
             return out
 
         return ('ModelChain: \n  ' + '\n  '.join(
-            (attr + ': ' + getmcattr(self, attr) for attr in attrs)))
+            ('{}: {}'.format(attr, getmcattr(self, attr)) for attr in attrs)))
 
     @property
     def orientation_strategy(self):
@@ -391,7 +391,7 @@ def infer_dc_model(self):
         elif set(['pdc0', 'gamma_pdc']) <= params:
             return self.pvwatts_dc
         else:
-            raise ValueError('could not infer DC model from ' +
+            raise ValueError('could not infer DC model from '
                              'system.module_parameters')
 
     def sapm(self):
@@ -455,7 +455,7 @@ def infer_ac_model(self):
         elif set(['pdc0']) <= module_params:
             return self.pvwatts_inverter
         else:
-            raise ValueError('could not infer AC model from ' +
+            raise ValueError('could not infer AC model from '
                              'system.inverter_parameters')
 
     def snlinverter(self):
@@ -502,7 +502,7 @@ def infer_aoi_model(self):
         elif set(['b']) <= params:
             return self.ashrae_aoi_loss
         else:
-            raise ValueError('could not infer AOI model from ' +
+            raise ValueError('could not infer AOI model from '
                              'system.module_parameters')
 
     def ashrae_aoi_loss(self):
@@ -547,7 +547,7 @@ def infer_spectral_model(self):
         if set(['A4', 'A3', 'A2', 'A1', 'A0']) <= params:
             return self.sapm_spectral_loss
         else:
-            raise ValueError('could not infer spectral model from ' +
+            raise ValueError('could not infer spectral model from '
                              'system.module_parameters')
 
     def first_solar_spectral_loss(self):

pvlib/pvsystem.py

@@ -156,7 +156,7 @@ def __repr__(self):
         attrs = ['name', 'surface_tilt', 'surface_azimuth', 'module',
                  'inverter', 'albedo', 'racking_model']
         return ('PVSystem: \n  ' + '\n  '.join(
-            (attr + ': ' + str(getattr(self, attr)) for attr in attrs)))
+            ('{}: {}'.format(attr, getattr(self, attr)) for attr in attrs)))
 
     def get_aoi(self, solar_zenith, solar_azimuth):
         """Get the angle of incidence on the system.
@@ -593,7 +593,7 @@ def __repr__(self):
             'surface_azimuth', 'module', 'inverter', 'albedo', 'racking_model'
                  ]
         return ('LocalizedPVSystem: \n  ' + '\n  '.join(
-            (attr + ': ' + str(getattr(self, attr)) for attr in attrs)))
+            ('{}: {}'.format(attr, getattr(self, attr)) for attr in attrs)))
 
 
 def systemdef(meta, surface_tilt, surface_azimuth, albedo, modules_per_string,
@@ -2084,8 +2084,8 @@ def adrinverter(v_dc, p_dc, inverter, vtol=0.10):
         See Notes for required keys.
 
     vtol : numeric, default 0.1
-        A unit-less fraction that determines how far the efficiency model is 
-        allowed to extrapolate beyond the inverter's normal input voltage 
+        A unit-less fraction that determines how far the efficiency model is
+        allowed to extrapolate beyond the inverter's normal input voltage
         operating range. 0.0 <= vtol <= 1.0
 
     Returns
@@ -2109,21 +2109,21 @@ def adrinverter(v_dc, p_dc, inverter, vtol=0.10):
     Column    Description
     =======   ============================================================
     p_nom     The nominal power value used to normalize all power values,
-              typically the DC power needed to produce maximum AC power 
+              typically the DC power needed to produce maximum AC power
               output, (W).
 
-    v_nom     The nominal DC voltage value used to normalize DC voltage 
-              values, typically the level at which the highest efficiency 
+    v_nom     The nominal DC voltage value used to normalize DC voltage
+              values, typically the level at which the highest efficiency
               is achieved, (V).
 
-    pac_max   The maximum AC output power value, used to clip the output 
+    pac_max   The maximum AC output power value, used to clip the output
               if needed, (W).
 
     ce_list   This is a list of 9 coefficients that capture the influence
               of input voltage and power on inverter losses, and thereby
               efficiency.
 
-    p_nt      ac-power consumed by inverter at night (night tare) to 
+    p_nt      ac-power consumed by inverter at night (night tare) to
               maintain circuitry required to sense PV array voltage, (W).
     =======   ============================================================
 

pvlib/test/test_modelchain.py

@@ -417,17 +417,18 @@ def test_basic_chain_altitude_pressure(sam_data):
     assert_series_equal(ac, expected, check_less_precise=2)
 
 
-def test_ModelChain___repr__(system, location):
-
-    strategy = 'south_at_latitude_tilt'
+@pytest.mark.parametrize('strategy, strategy_str', [
+    ('south_at_latitude_tilt', 'south_at_latitude_tilt'),
+    (None, 'None')])  # GitHub issue 352
+def test_ModelChain___repr__(system, location, strategy, strategy_str):
 
     mc = ModelChain(system, location, orientation_strategy=strategy,
                     name='my mc')
 
     expected = '\n'.join([
         'ModelChain: ',
         '  name: my mc',
-        '  orientation_strategy: south_at_latitude_tilt',
+        '  orientation_strategy: ' + strategy_str,
         '  clearsky_model: ineichen',
         '  transposition_model: haydavies',
         '  solar_position_method: nrel_numpy',
@@ -442,7 +443,6 @@ def test_ModelChain___repr__(system, location):
 
     assert mc.__repr__() == expected
 
-
 @requires_scipy
 def test_weather_irradiance_input(system, location):
     """Test will raise a warning and should be removed in future versions."""

pvlib/test/test_pvsystem.py

@@ -570,7 +570,7 @@ def test_PVSystem_sapm_celltemp():
 
 def test_adrinverter(sam_data):
     inverters = sam_data['adrinverter']
-    testinv = 'Ablerex_Electronics_Co___Ltd___' + \
+    testinv = 'Ablerex_Electronics_Co___Ltd___' \
               'ES_2200_US_240__240_Vac__240V__CEC_2011_'
     vdcs = pd.Series([135, 154, 390, 420, 551])
     pdcs = pd.Series([135, 1232, 1170, 420, 551])
@@ -582,7 +582,7 @@ def test_adrinverter(sam_data):
 
 def test_adrinverter_vtol(sam_data):
     inverters = sam_data['adrinverter']
-    testinv = 'Ablerex_Electronics_Co___Ltd___' + \
+    testinv = 'Ablerex_Electronics_Co___Ltd___' \
               'ES_2200_US_240__240_Vac__240V__CEC_2011_'
     vdcs = pd.Series([135, 154, 390, 420, 551])
     pdcs = pd.Series([135, 1232, 1170, 420, 551])
@@ -594,7 +594,7 @@ def test_adrinverter_vtol(sam_data):
 
 def test_adrinverter_float(sam_data):
     inverters = sam_data['adrinverter']
-    testinv = 'Ablerex_Electronics_Co___Ltd___' + \
+    testinv = 'Ablerex_Electronics_Co___Ltd___' \
               'ES_2200_US_240__240_Vac__240V__CEC_2011_'
     vdcs = 154.
     pdcs = 1232.

pvlib/tmy.py

@@ -158,8 +158,8 @@ def readtmy3(filename=None, coerce_year=None, recolumn=True):
         try:
             filename = _interactive_load()
         except:
-            raise Exception('Interactive load failed. Tkinter not supported ' +
-                            'on this system. Try installing X-Quartz and ' +
+            raise Exception('Interactive load failed. Tkinter not supported '
+                            'on this system. Try installing X-Quartz and '
                             'reloading')
 
     head = ['USAF', 'Name', 'State', 'TZ', 'latitude', 'longitude', 'altitude']
@@ -467,20 +467,17 @@ def _read_tmy2(string, columns, hdr_columns, fname):
                     try:
                         val = float(val)
                     except:
-                        raise Exception('WARNING: In' + fname +
-                                        ' Read value is not an integer " ' +
-                                        val + ' " ')
+                        raise Exception('WARNING: In {} Read value is not an '
+                                        'integer " {} " '.format(fname, val))
                 elif marker[-1] == 's':
                     try:
                         val = str(val)
                     except:
-                        raise Exception('WARNING: In' + fname +
-                                        ' Read value is not a string" ' +
-                                        val + ' " ')
+                        raise Exception('WARNING: In {} Read value is not a '
+                                        'string " {} " '.format(fname, val))
                 else:
-                    raise Exception('WARNING: In' + __name__ +
-                                    'Improper column DataFrame " %' +
-                                    marker + ' " ')
+                    raise Exception('WARNING: In {} Improper column DataFrame '
+                                    '" %{} " '.format(__name__, marker))
 
                 part.append(val)
 

pvlib/tracking.py

@@ -59,7 +59,7 @@ def __init__(self, axis_tilt=0, axis_azimuth=0,
     def __repr__(self):
         attrs = ['axis_tilt', 'axis_azimuth', 'max_angle', 'backtrack', 'gcr']
         sat_repr = ('SingleAxisTracker: \n  ' + '\n  '.join(
-            (attr + ': ' + str(getattr(self, attr)) for attr in attrs)))
+            ('{}: {}'.format(attr, getattr(self, attr)) for attr in attrs)))
         # get the parent PVSystem info
         pvsystem_repr = super(SingleAxisTracker, self).__repr__()
         # remove the first line (contains 'PVSystem: \n')
@@ -196,8 +196,8 @@ def __repr__(self):
         attrs = ['latitude', 'longitude', 'altitude', 'tz']
         return ('Localized' +
                 super(LocalizedSingleAxisTracker, self).__repr__() + '\n  ' +
-                '\n  '.join(
-                    (attr + ': ' + str(getattr(self, attr)) for attr in attrs)))
+                '\n  '.join(('{}: {}'.format(attr, getattr(self, attr))
+                             for attr in attrs)))
 
 
 def singleaxis(apparent_zenith, apparent_azimuth,
@@ -280,7 +280,7 @@ def singleaxis(apparent_zenith, apparent_azimuth,
 
     pvl_logger.debug('tracking.singleaxis')
 
-    pvl_logger.debug('axis_tilt=%s, axis_azimuth=%s, max_angle=%s, ' +
+    pvl_logger.debug('axis_tilt=%s, axis_azimuth=%s, max_angle=%s, '
                      'backtrack=%s, gcr=%.3f',
                      axis_tilt, axis_azimuth, max_angle, backtrack, gcr)
 
@@ -310,7 +310,7 @@ def singleaxis(apparent_zenith, apparent_azimuth,
         pd.util.testing.assert_index_equal(apparent_azimuth.index,
                                            apparent_zenith.index)
     except AssertionError:
-        raise ValueError('apparent_azimuth.index and ' +
+        raise ValueError('apparent_azimuth.index and '
                          'apparent_zenith.index must match.')
 
     times = apparent_azimuth.index