Deprecate/raise error for PVSystem "pass-through" properties #1196
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,36 @@ def f(*args, **kwargs): | |
| return f | ||
|
|
||
|
|
||
| def _check_deprecated_passthrough(func): | ||
| """ | ||
| Decorator to warn or error when getting and setting the "pass-through" | ||
| PVSystem properties that have been moved to Array. Emits a warning for | ||
| PVSystems with only one Array and raises an error for PVSystems with | ||
| more than one Array. | ||
| """ | ||
|
|
||
| @functools.wraps(func) | ||
| def wrapper(self, *args, **kwargs): | ||
| pvsystem_attr = func.__name__ | ||
| class_name = self.__class__.__name__ # PVSystem or SingleAxisTracker | ||
| overrides = { # some Array attrs aren't the same as PVSystem | ||
| 'strings_per_inverter': 'strings', | ||
| } | ||
| array_attr = overrides.get(pvsystem_attr, pvsystem_attr) | ||
| alternative = f'{class_name}.arrays[i].{array_attr}' | ||
|
|
||
| if len(self.arrays) > 1: | ||
| raise AttributeError( | ||
| f'{class_name}.{pvsystem_attr} not supported for multi-array ' | ||
| f'systems. Use {alternative} instead.') | ||
|
||
|
|
||
| wrapped = deprecated('0.9', alternative=alternative, removal='0.10', | ||
| name=f"{class_name}.{pvsystem_attr}")(func) | ||
| return wrapped(self, *args, **kwargs) | ||
|
|
||
| return wrapper | ||
|
|
||
|
|
||
| # not sure if this belongs in the pvsystem module. | ||
| # maybe something more like core.py? It may eventually grow to | ||
| # import a lot more functionality from other modules. | ||
|
|
@@ -1019,72 +1049,86 @@ def pvwatts_ac(self, pdc): | |
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def module_parameters(self): | ||
| return tuple(array.module_parameters for array in self.arrays) | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def module(self): | ||
| return tuple(array.module for array in self.arrays) | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def module_type(self): | ||
| return tuple(array.module_type for array in self.arrays) | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def temperature_model_parameters(self): | ||
| return tuple(array.temperature_model_parameters | ||
| for array in self.arrays) | ||
|
|
||
| @temperature_model_parameters.setter | ||
|
Member
There was a problem hiding this comment. The pvlib-python/pvlib/modelchain.py Lines 503 to 510 in 7eae1fc For that matter we could implement a pvlib-python/pvlib/modelchain.py Lines 491 to 501 in 7eae1fc I don't know that one approach is better than another.
Member
Author
There was a problem hiding this comment. I'm not following the issue here, can you expand on what's different about the
Member
There was a problem hiding this comment. After trying to come up with an example, I'm realizing the problem is a little different than I first described. There are currently setters for only 4 properties:
The remaining properties do not contain setters. At first I thought this meant that for all of the other properties, a user could set something on a In [5]: system = PVSystem([Array()])
In [6]: system.albedo = 0.3
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-6-d89b04b86f95> in <module>
----> 1 system.albedo = 0.3
AttributeError: can't set attributeSo the issue is that the user can set some but not all of the deprecated attributes. I'd prefer if they could set none or all. I brought up the
Member
Author
There was a problem hiding this comment.
I see, and agree it should be consistent. I think omitting the setters was a (probably unintentional?) breaking change, so I think being able to set all (with a deprecation warning) makes sense. I've added setters where they were missing. |
||
| @_check_deprecated_passthrough | ||
| def temperature_model_parameters(self, value): | ||
| for array in self.arrays: | ||
| array.temperature_model_parameters = value | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def surface_tilt(self): | ||
| return tuple(array.surface_tilt for array in self.arrays) | ||
|
|
||
| @surface_tilt.setter | ||
| @_check_deprecated_passthrough | ||
| def surface_tilt(self, value): | ||
| for array in self.arrays: | ||
| array.surface_tilt = value | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def surface_azimuth(self): | ||
| return tuple(array.surface_azimuth for array in self.arrays) | ||
|
|
||
| @surface_azimuth.setter | ||
| @_check_deprecated_passthrough | ||
| def surface_azimuth(self, value): | ||
| for array in self.arrays: | ||
| array.surface_azimuth = value | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def albedo(self): | ||
| return tuple(array.albedo for array in self.arrays) | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def racking_model(self): | ||
| return tuple(array.racking_model for array in self.arrays) | ||
|
|
||
| @racking_model.setter | ||
| @_check_deprecated_passthrough | ||
| def racking_model(self, value): | ||
| for array in self.arrays: | ||
| array.racking_model = value | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def modules_per_string(self): | ||
| return tuple(array.modules_per_string for array in self.arrays) | ||
|
|
||
| @property | ||
| @_unwrap_single_value | ||
| @_check_deprecated_passthrough | ||
| def strings_per_inverter(self): | ||
| return tuple(array.strings for array in self.arrays) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the alphabetization of the list preceding this one and would prefer to see that here too.