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

Finish transition from tox to Nox for current continuous integration checks and weekly tests #2694

Draft
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

namurphy
Copy link
Member

@namurphy namurphy commented May 16, 2024

This PR concludes the switchover from tox to Nox. I wanted to finish this ahead of the PlasmaPy Summer School so that participants won't need to learn how to work with tox (or more specifically, tox.ini).

The changes in this PR include:

  • 📚 I added a Nox session to build docs against the development versions of Sphinx, sphinx_rtd_theme, and nbsphinx.
  • 🧪 I added a Nox session to run tests against the development versions of NumPy, Astropy, xarray, lmfit (since we had a breaking change come up recently), and pandas.
  • 🧪 I switched over weekly tests to entirely using Nox sessions
  • ♻️ I consolidated the separate workflows for building docs and running tests in CI to a single workflow.
  • ✂️ I deleted tox.ini in its entirely, and there was much rejoicing. Using tox has overall been great, except for the INI-formatted configuration files! (We still have mypy.ini...but I'm hoping that's only temporary.)
  • ✂️ I removed requirements.txt, as inspired by Requirements files need to be rethought #2647. It's not being used in CI anymore or by any Nox sessions, so we don't need it. Plus, IDEs are much more adept at getting requirements information from pyproject.toml than they used to be.
  • ✂️ I removed the requirements files used only by tox environments. These have been superseded by the requirements files used by Nox sessions.
  • ✨ I made it so that weekly tests can be run for PRs that are labeled with "run weekly tests in CI".

Some notes:

  • There were a few checks that I wasn't able to get working.
    • 🧪 Testing against the lowest-direct versions of dependencies for Python 3.11 & 3.12.
    • 🧪 Testing against the development version of h5py.
    • 🌠 Building the docs on Windows or macOS, since the sudo apt install step for pandoc and graphviz is specific to Ubuntu.
    • 🦺 I added mypy to the tests dependency set, primarily because then we won't have requirements information defined in noxfile.py that we have to update.

Remaining tasks

  • Clean up noxfile.py
  • Add more changelog entries
  • Possibly spin off some of the smaller, more isolated changes into distinct PRs.

@github-actions github-actions bot added testing CI Related to continuous integration python Pull requests that update Python code nox Related to the nox automation software labels May 16, 2024
@github-actions github-actions bot added the maintenance General updates to package infrastructure label May 16, 2024
@github-actions github-actions bot added the GitHub Actions A continuous integration platform for automating tests and other tasks (see .github/ directory) label May 16, 2024
Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.23%. Comparing base (5a39092) to head (2c8dde4).

Current head 2c8dde4 differs from pull request most recent head bc0a7f6

Please upload reports for the commit bc0a7f6 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2694   +/-   ##
=======================================
  Coverage   95.23%   95.23%           
=======================================
  Files         104      104           
  Lines        9399     9399           
  Branches     2155     2155           
=======================================
  Hits         8951     8951           
  Misses        272      272           
  Partials      176      176           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

To make sure that mypy is run consistently across platforms.
I updated the conditions when this workflow happens so that it is only done
as a cron job for PlasmaPy/PlasmaPy@main, but will still run on PRs with the
appropriate label, and by workflow dispatch.
@github-actions github-actions bot added linters Code linters and autoformatters documentation infrastructure labels May 16, 2024
This makes it so that mypy checks are not being performed with pinned
requirements. If we run into problems where mypy checks suddenly start
breaking due to breaking changes in mypy, then we can revisit this.
@github-actions github-actions bot added the packaging Related to packaging or distribution label May 16, 2024
@namurphy namurphy changed the title Add nox session to build documentation against unreleased versions of dependencies Finish transition from tox to Nox for continuous integration checks May 17, 2024
@@ -12,12 +16,10 @@ on:
workflow_dispatch:

env:
DOC_TROUBLESHOOTING: "Warnings like 'reference target not found' occur when Sphinx tries to interpret text as a Python object, but is unable to do so. This warning can often be fixed by surrounding text in double back ticks instead of single back ticks (e.g., by changing `y` to ``y``) so that it gets formatted as an in-line literal. For more information about addressing documentation build failures, please check out the documentation troubleshooting guide at: https://docs.plasmapy.org/en/latest/contributing/doc_guide.html#troubleshooting"
Copy link
Member Author

Choose a reason for hiding this comment

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

🛗 I moved this into the Nox session (with some edits).

- name: Install nox and uv
- name: Install graphviz and pandoc for documentation builds
if: startsWith(matrix.name, 'Documentation')
run: sudo apt-get install graphviz pandoc
Copy link
Member Author

@namurphy namurphy May 17, 2024

Choose a reason for hiding this comment

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

⚠️ This command only works for Ubuntu, so we would have to change this later on if we want to build docs on Windows or macOS in CI. (It would be nice to have that capability, but I'm less worried about building docs across platforms than I am about running tests.)

Comment on lines -103 to -145

documentation:

name: Documentation, Python 3.12, Ubuntu
runs-on: ubuntu-latest

steps:

- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Install graphviz and pandoc
run: sudo apt-get install graphviz pandoc

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.12'
cache: pip

- name: Install nox uv
run: python -m pip install --progress-bar off --upgrade nox uv

- name: Cache
uses: actions/cache@v4
with:
path: |
.nox
key: docs-${{ runner.os }}-${{ hashFiles('requirements.txt') }}

- name: Build documentation with Sphinx
run: nox -e docs -- -q

- name: Print troubleshooting information on failure
if: ${{ failure() }}
run: echo -e $DOC_TROUBLESHOOTING
Copy link
Member Author

Choose a reason for hiding this comment

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

There was a lot of duplication in the workflows for building docs and running tests, so I merged them together above.

Comment on lines +20 to +23
if: >-
github.event_name == 'workflow_dispatch' ||
(github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'perform linkcheck in CI')) ||
(github.event_name == 'schedule' && github.repository == 'PlasmaPy/PlasmaPy' && github.ref == 'refs/heads/main')
Copy link
Member Author

@namurphy namurphy May 17, 2024

Choose a reason for hiding this comment

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

📝 This is saying that this workflow should only be run:

  • If we manually trigger it (via workflow dispatch)
  • It's a pull request that has been labeled with "perform linkcheck in CI"
  • As a cron job if it's on the main branch of PlasmaPy/PlasmaPy.

My motivation here is to avoid cron jobs being run on forks, which can waste a lot of computing resources.

I'll be honest...ChatGPT helped me write this one.

@namurphy namurphy changed the title Finish transition from tox to Nox for continuous integration checks Finish transition from tox to Nox for current continuous integration checks and weekly tests May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Related to continuous integration documentation infrastructure GitHub Actions A continuous integration platform for automating tests and other tasks (see .github/ directory) linters Code linters and autoformatters maintenance General updates to package infrastructure nox Related to the nox automation software packaging Related to packaging or distribution python Pull requests that update Python code requirements Related to updating requirements run weekly tests in CI Add this label to PRs to additionally run weekly tests. testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant