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

Avoid duplicate / inconsistent environment descriptors #71

Closed
brian-rose opened this issue Apr 28, 2021 · 6 comments · Fixed by #72
Closed

Avoid duplicate / inconsistent environment descriptors #71

brian-rose opened this issue Apr 28, 2021 · 6 comments · Fixed by #72
Labels
infrastructure Infrastructure related issue

Comments

@brian-rose
Copy link
Member

As discussed in #68 and #70, we currently have two different descriptions of the sphinx build environment:

  • requirements.txt is used by pip
    • on readthedocs for automated testing of PRs
    • on GitHub Actions to build and deploy the actual portal site
  • ci/environment.yml is used by conda, which we recommend for local building in our Contributor's Guide

We have no automated way to checking for inconsistencies between these two, which recently led to a local build error #68 . Clearly it's not ideal that our CI tests are not catching this.

A few paths forward:

  1. Modify both GitHub Actions and readthedocs to use conda instead of pip, making sure they are building in a way that is consistent with our own local build instructions (my preference)
  2. Change our Contributor's Guide to suggest using pip instead of conda (I don't think this is a good idea)
  3. Find a CI tool that will automatically check for inconsistencies between the two environment files? (I don't know how to do this, but even if I did, it seems like a band-aid solution compared to 1.)

Thoughts from the Infrastructure team?

@brian-rose brian-rose added the infrastructure Infrastructure related issue label Apr 28, 2021
@ktyle
Copy link
Contributor

ktyle commented Apr 28, 2021

I agree that we should use conda instead of pip whenever possible ... still run into situations where some packages don't exist on conda-forge, but can be found/installed via pip.

@andersy005 andersy005 mentioned this issue Apr 29, 2021
@dcamron
Copy link
Contributor

dcamron commented Apr 29, 2021

Just want to hop in to mention that if we prefer/need to maintain one file compatible with both pip and conda (as long as we confirm every spec is available on both), you can conda create/install/update and conda env create/update with text specification files like requirements.txt, e.g. conda create -n <env> -c conda-forge --file requirements.txt. This is how @dopplershift has set up MetPy ci and contributors guide.

@dopplershift
Copy link
Contributor

I'll also add that using requirements.txt with Conda has allowed us to pin versions of packages to avoid being broken by updates--instead we get to control when we move to new versions, rather than just potentially getting new versions whenever we do a new build. Instead we get PRs that update each version and let us see what the ramifications of the update are.

@ktyle If we have packages important for our ecosystem that are not yet available on conda-forge, I'm happy to help try to get them added.

@brian-rose
Copy link
Member Author

@dopplershift is that specific to using requirements.txt? Can't you just as easily specify package versions in environment.yml?

@dopplershift
Copy link
Contributor

If you want to use Dependabot to automatically issue Pull Requests to update the versions when new versions are released, then unfortunately only requirements.txt (and other such files) is supported right now (dependabot/dependabot-core#2227). On MetPy we decided eliminating random, unrelated PR builds failing was worth having the use of Conda with our dependency files being a bit more clunky.

@brian-rose
Copy link
Member Author

I see, thanks!

Do we think this would be an asset for any of our Pythia repos? The dependencies are likely to get more complicated as the projects grow.

@andersy005 already submitted #72 that converts all our builds to conda. We could modify those to use requirements.txt rather than environment.yml

@kmpaul kmpaul closed this as completed in #72 May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Infrastructure related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants