Skip to content

Conversation

@kandersolar
Copy link
Member

@kandersolar kandersolar commented Jul 29, 2022

  • [ ] Closes #xxxx
  • I am familiar with the contributing guidelines
  • [ ] Tests added
  • [ ] Updates entries in docs/sphinx/source/reference for API changes.
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/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`).
  • [ ] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

I expect Windows tests to fail until mamba 0.25.1 is released; see conda-forge/micromamba-feedstock#90 and mamba-org/mamba#1828. I'll leave this PR as a draft until that issue is resolved and everything is passing here. The mamba fix is deployed, so those tests are passing. However the 3.6 remote-data tests will continue to fail (and the 3.10 won't run at all) because it's a pull_request_target action for security reasons and still using the configuration on master. There's nothing to "fix" there, it just means we need to manually inspect the changes to the remote-data configuration instead of relying on the CI.

We should probably also take a look at uncommenting some items in the conda environment files. I've not done that here. The new 3.10 environment is essentially just a copy of the 3.9 environment.

@kandersolar kandersolar added this to the 0.9.2 milestone Jul 29, 2022
@kandersolar kandersolar mentioned this pull request Jul 29, 2022
9 tasks
@kandersolar kandersolar added the remote-data triggers --remote-data pytests label Jul 29, 2022
@kandersolar kandersolar marked this pull request as ready for review July 29, 2022 18:55
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @kanderso-nrel. Should we handle the uncommenting in another PR or try it here?

I think we've cleared out all of the documentation's explicit references to python version compatibility. And setup.py classifier is just python 3. ✅

We should update the upload to anaconda.org part of the release procedures once this is merged.

Do we need to do anything with pvlib-benchmarker? I don't think so but not confident.

@kandersolar
Copy link
Member Author

Should we handle the uncommenting in another PR or try it here?

Mild preference for another PR

We should update the upload to anaconda.org part of the release procedures once this is merged.

+1, and in hindsight I shouldn't have added 3.10 to that list before adding it to the test matrix. But per your suggestion a few months ago, maybe it's time to stop updating the pvlib channel altogether?

Do we need to do anything with pvlib-benchmarker? I don't think so but not confident.

I also don't think so, at least not immediately. The only thing is that changing the python version will create yet another disconnect/variant in the plots. I wonder if we should periodically re-run the benchmarks using the current environments so that comparisons across time are apples to apples.

@wholmgren
Copy link
Member

wholmgren commented Jul 29, 2022

But per your suggestion a few months ago, maybe it's time to stop updating the pvlib channel altogether?

I think we should first resolve conda-forge/pvlib-python-feedstock#30

@pvlib/pvlib-maintainer please merge if you also approve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

remote-data triggers --remote-data pytests testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants