Skip to content
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

Test and CI updates #129

Merged
merged 10 commits into from
Feb 16, 2022
Merged

Test and CI updates #129

merged 10 commits into from
Feb 16, 2022

Conversation

kandersolar
Copy link
Member

@kandersolar kandersolar commented Jan 31, 2022

Description

This PR:

  • Drops python 3.6 and adds python 3.9, 3.10 to CI tests
  • Some tests currently use deprecated pvlib functions. This PR adds equivalent tests using non-deprecated pvlib functions. Each set of tests is marked to only run if the appropriate pvlib version is installed. The older tests can be deleted when we eventually drop support for pvlib<0.9.0 down the line.
  • Resolves a few pandas deprecation warnings in the test functions. Most of the pandas warnings are not easily fixed and are not addressed here.

Because this PR doesn't add any new tests using multiple Arrays, it only partially addresses #116.

Checklist

  • [ ] Closes #xxx
  • [ ] Added new API functions to docs/api.rst
  • [ ] Clearly documented all new API functions with PEP257 and numpydoc compliant docstrings
  • Adds description and name entries in the appropriate "what's new" file
    in docs/whatsnew
    for all changes. Includes link to the GitHub Issue with :issue:`num`
    or this Pull Request with :pull:`num`. Includes contributor name
    and/or GitHub username (link with :ghuser:`user`).
  • [ ] Non-API functions clearly documented with docstrings or comments as necessary
  • [ ] Added tests to cover all new or modified code
  • Pull request is nearly complete and ready for detailed review

@kandersolar kandersolar added ci Continuous Integration tests Something is wrong with the test suite labels Jan 31, 2022
@kandersolar
Copy link
Member Author

Ok, ready for review.

A bit of context for the last commit: bfc9390 fixes an issue with scipy v1.8.0 (released Feb 5, 2022), which includes a change where scipy.stats.linregress raises an error if all x values are identical (scipy/scipy#14757). In most cases this wouldn't matter, except some of the tests have all identical y values and it seems that quadratic_r2 passes in (y, x) instead of the (x, y) that scipy.stats.linregress requires:

_, _, correlation, _, _ = scipy.stats.linregress(
y, quadratic(x)
)

However, switching to the correct parameter order doesn't get all the tests passing: quadratic(x), when y has zero variance, returns identical values and causes the scipy error again. I've added a special case for this to return r2=0 when y has no variability.

@kandersolar kandersolar marked this pull request as ready for review February 14, 2022 19:27
This was referenced Feb 14, 2022
@cwhanse
Copy link
Member

cwhanse commented Feb 16, 2022

OK to merge when the whatsnew conflict is resolved.

@cwhanse
Copy link
Member

cwhanse commented Feb 16, 2022

Build fails after merging #127. But it appears the failures will be fixed with this PR.

@kandersolar
Copy link
Member Author

Build fails after merging #127. But it appears the failures will be fixed with this PR.

Tests here are green, so I expect so.

@kandersolar kandersolar merged commit 1d43c1a into pvlib:master Feb 16, 2022
@kandersolar kandersolar deleted the ci-updates branch February 16, 2022 20:13
@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
ci Continuous Integration tests Something is wrong with the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants