Repository: pvlib/pvlib-python
Pull request #330 (2017-06-02 10:33:32)

Change method of multi-inheritance to allow kwargs in parent class of LocalizedPVSystem and LocalizedSingleAxisTracker
If keyword arguments are present a `TypeError` will be raised, so it is better to remove it from the `super` call.

As I normally initialise classes using a parameter dictionary, so it often happens that I will have additional keys. Therefore it is convenient if the PVSystem class takes `**kwargs` but they should not be passed on.

uvchik: My fault, so i reverted the commit.

For me it would be nice to make it possible to add additional parameters to the dictionary and pass it using the double star operator (see below). But may be it was a deliberate decision to forbid superfluous parameter.

```python
location = {
        'altitude': 34,
        'latitude': latlon.loc[int(key[2:]), 'st_y'],
        'longitude': latlon.loc[int(key[2:]), 'st_x']}

system = {
    'module_parameters': sandia_modules[module_name],
    'inverter_parameters': sapm_inverters[inverter_name],
    'surface_azimuth': 180,
    'surface_tilt': 30,
    'albedo': 0.2,
    'additional_information': 'something' 
    }
mc = ModelChain(PVSystem(**system), Location(**location))
```

If you want to allow additional parameters in this class I could think about, how to did, if not just close this PR. I do not use it all the time, but I like it especially for system parameters.
wholmgren: I've run into this issue, as well, but haven't given much thought to the best approach. I am open to changing things, though.
uvchik: What I read about multiple inheritance, one can call the init-methods of the parent classes explicitly to avoid such behaviour. After I have changed everything, all tests work fine, except of one. But I think the test is wrong and not my changes, because `my name` is passed to the method and so the expected name attribute should be `my name` and not `None`.

@wholmgren May you check, if I changed the test correctly (commit: 37d84ec).
wholmgren: Thanks. After (re)reading a bunch of stuff on multiple inheritance in python, I think your proposal is a better way to handle these classes.

I agree that the test asserts the wrong result. It looks like the ``PVSystem.__init__`` consumes the ``name`` kwarg before it gets to ``Location``, then the ``Location__init__`` sets the ``name`` to ``None``. Your code passes all kwargs to both ``__init__`` methods, so the composite object ends up with the right attribute value regardless of the initialization order.

Can you start a 0.4.6 whats new file and describe this fix?
uvchik: Thank you for your feedback.

I added a whatsnew file as `.rst` as it is a reStructuredText file. This makes file handling a little easier.

I hope I described my changes comprehensible.
uvchik: Is it okay like this?
wholmgren: Yes, thanks!

Patch:
diff --git a/docs/sphinx/source/conf.py b/docs/sphinx/source/conf.py
index 21d3cd5..891386e 100644
--- a/docs/sphinx/source/conf.py
+++ b/docs/sphinx/source/conf.py
@@ -100,7 +100,7 @@ release = version
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
-exclude_patterns = []
+exclude_patterns = ['whatsnew/*']
 
 # The reST default role (used for this markup: `text`) to use for all
 # documents.
diff --git a/docs/sphinx/source/whatsnew.rst b/docs/sphinx/source/whatsnew.rst
index 287e21e..daff232 100644
--- a/docs/sphinx/source/whatsnew.rst
+++ b/docs/sphinx/source/whatsnew.rst
@@ -6,6 +6,7 @@ What's New
 
 These are new features and improvements of note in each release.
 
+.. include:: whatsnew/v0.4.6.rst
 .. include:: whatsnew/v0.4.5.txt
 .. include:: whatsnew/v0.4.4.txt
 .. include:: whatsnew/v0.4.3.txt
diff --git a/docs/sphinx/source/whatsnew/v0.4.6.rst b/docs/sphinx/source/whatsnew/v0.4.6.rst
new file mode 100644
index 0000000..e123236
--- /dev/null
+++ b/docs/sphinx/source/whatsnew/v0.4.6.rst
@@ -0,0 +1,31 @@
+.. _whatsnew_0460:
+
+v0.4.6 ()
+---------
+
+
+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`)
+
+
+Enhancements
+~~~~~~~~~~~~
+
+
+API Changes
+~~~~~~~~~~~
+
+
+Documentation
+~~~~~~~~~~~~~
+
+
+Contributors
+~~~~~~~~~~~~
+
+* Will Holmgren
+* Uwe Krien
diff --git a/pvlib/location.py b/pvlib/location.py
index 217667b..4c51d9e 100644
--- a/pvlib/location.py
+++ b/pvlib/location.py
@@ -79,10 +79,6 @@ class Location(object):
 
         self.name = name
 
-        # needed for tying together Location and PVSystem in LocalizedPVSystem
-        # if LocalizedPVSystem signature is reversed
-        # super(Location, self).__init__(**kwargs)
-
     def __repr__(self):
         attrs = ['name', 'latitude', 'longitude', 'altitude', 'tz']
         return ('Location: \n  ' + '\n  '.join(
diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py
index 823fd8b..daeecd4 100644
--- a/pvlib/pvsystem.py
+++ b/pvlib/pvsystem.py
@@ -149,9 +149,6 @@ class PVSystem(object):
 
         self.racking_model = racking_model
 
-        # needed for tying together Location and PVSystem in LocalizedPVSystem
-        super(PVSystem, self).__init__(**kwargs)
-
     def __repr__(self):
         attrs = ['name', 'surface_tilt', 'surface_azimuth', 'module',
                  'inverter', 'albedo', 'racking_model']
@@ -584,7 +581,8 @@ class LocalizedPVSystem(PVSystem, Location):
                           list(loc_dict.items()) +
                           list(kwargs.items()))
 
-        super(LocalizedPVSystem, self).__init__(**new_kwargs)
+        PVSystem.__init__(self, **new_kwargs)
+        Location.__init__(self, **new_kwargs)
 
     def __repr__(self):
         attrs = [
diff --git a/pvlib/tracking.py b/pvlib/tracking.py
index 64988e1..6eb8360 100644
--- a/pvlib/tracking.py
+++ b/pvlib/tracking.py
@@ -38,7 +38,6 @@ class SingleAxisTracker(PVSystem):
         pvsystem_repr = '\n'.join(pvsystem_repr.split('\n')[1:])
         return sat_repr + '\n' + pvsystem_repr
 
-
     def singleaxis(self, apparent_zenith, apparent_azimuth):
         tracking_data = singleaxis(apparent_zenith, apparent_azimuth,
                                    self.axis_tilt, self.axis_azimuth,
@@ -162,14 +161,15 @@ class LocalizedSingleAxisTracker(SingleAxisTracker, Location):
                           list(loc_dict.items()) +
                           list(kwargs.items()))
 
-        super(LocalizedSingleAxisTracker, self).__init__(**new_kwargs)
+        SingleAxisTracker.__init__(self, **new_kwargs)
+        Location.__init__(self, **new_kwargs)
 
     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)))
+                super(LocalizedSingleAxisTracker, self).__repr__() + '\n  ' +
+                '\n  '.join(
+                    (attr + ': ' + str(getattr(self, attr)) for attr in attrs)))
 
 
 def singleaxis(apparent_zenith, apparent_azimuth,


Test patch:
diff --git a/pvlib/test/test_pvsystem.py b/pvlib/test/test_pvsystem.py
index 3a28fef..8416018 100644
--- a/pvlib/test/test_pvsystem.py
+++ b/pvlib/test/test_pvsystem.py
@@ -751,7 +751,7 @@ def test_LocalizedPVSystem___repr__():
                                                   inverter='blarg',
                                                   name='my name')
 
-    expected = 'LocalizedPVSystem: \n  name: None\n  latitude: 32\n  longitude: -111\n  altitude: 0\n  tz: UTC\n  surface_tilt: 0\n  surface_azimuth: 180\n  module: blah\n  inverter: blarg\n  albedo: 0.25\n  racking_model: open_rack_cell_glassback'
+    expected = 'LocalizedPVSystem: \n  name: my name\n  latitude: 32\n  longitude: -111\n  altitude: 0\n  tz: UTC\n  surface_tilt: 0\n  surface_azimuth: 180\n  module: blah\n  inverter: blarg\n  albedo: 0.25\n  racking_model: open_rack_cell_glassback'
 
     assert localized_system.__repr__() == expected