Fix issue with window_length in reno clearsky detection#221
Fix issue with window_length in reno clearsky detection#221kandersolar merged 2 commits intopvlib:mainfrom
window_length in reno clearsky detection#221Conversation
|
python 3.8 CI failures should be ignored; we'll stop using 3.8 in #220. |
I think I vote we leave @kperrynrel your opinion? I mostly don't want to break any workflow for PVFleets. |
|
@cwhanse @kandersolar thanks for checking in with us on this. Just took a look at our workflows and we're using the clearsky call associated with the Location() class in pvlib, which doesn't appear to use pvlib's clearsky.detect_clearsky() function so no issues expected on our end for changing this logic up. I'd vote for a separate PR for adding infer_limits since it's a modification and this is a bug fix. |
|
@kperrynrel the pvlib |
|
@cwhanse we do not actually apply a clearsky filter in the routine. We do use the fixed_nrel() and tracking_nrel() functions in pvanalytics to identify sunny/clearsky days as a whole, and that is mainly what is used in lieu of any clearsky filter. |
|
@kandersolar I think this PR is fine as is, or we could add a test to spy on pvlib's function call. |
|
In it goes! |
docs/api.rst.in
docs/whatsnewfor all changes. Includes link to the GitHub Issue with
:issue:`num`or this Pull Request with
:pull:`num`. Includes contributor nameand/or GitHub username (link with
:ghuser:`user`).renowas accidentally passingwindow_lengthto theinfer_limitsparameter. That means the window length was not being provided to the underlying pvlib function and instead the default was used. It also means that, since non-zero numbers are truthy, presumably this function has effectively hadinfer_limits=True.@cwhanse should
infer_limits=Truebe retained here?