Skip to content

Tests for clearsky.reno fail with pvlib 0.8.1 #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wfvining opened this issue Apr 6, 2021 · 4 comments · Fixed by #128
Closed

Tests for clearsky.reno fail with pvlib 0.8.1 #105

wfvining opened this issue Apr 6, 2021 · 4 comments · Fixed by #128
Milestone

Comments

@wfvining
Copy link
Collaborator

wfvining commented Apr 6, 2021

PVLib 0.8.1 changed the labeling of clear-sky periods (pvlib/pvlib-python#1074) which is resulting in a test failures in test_clearsky.py.

The change in pvlib labels windows on the center by default, while previous behavior labeled on the left. We probably want to accept the new default behavior from PVLib (updating the tests to match) and expose the new align parameter in pvanalytics.clearsky.reno.

@cwhanse
Copy link
Member

cwhanse commented Apr 6, 2021

The change in pvlib labels windows on the left by default, while previous behavior used centered labels.

Other way around; before v0.8.1, pvlib used left labels. pvlib v0.8.1 uses centered labels.

@wfvining
Copy link
Collaborator Author

wfvining commented Apr 6, 2021

Other way around; before v0.8.1, pvlib used left labels. pvlib v0.8.1 uses centered labels.

Thanks, updated.

@wfvining
Copy link
Collaborator Author

Actually, the alignment is not exposed in the pvlib API. We should probably just accept the centered labels here, and remove/rewrite tests that relied on left-aligned labels. I'm thinking that my approach to testing this function was flawed to begin with, it might be better to mock the pvlib function and check that the window/scale are computed correctly and passed the pvlib function (otherwise we are spending a bunch of effort testing code that isn't even part of pvanalytics).

@cwhanse
Copy link
Member

cwhanse commented Jun 4, 2021

Tests for clearsky.reno fail because of a bug in pvlib.clearsky #pvlib/pvlib-python#1241

kandersolar added a commit to kandersolar/pvanalytics that referenced this issue Jan 27, 2022
kandersolar added a commit that referenced this issue Jan 27, 2022
@kandersolar kandersolar added this to the v0.1.1 milestone Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants