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

Switch to tox #5184

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
IRIS_TEST_DATA_PATH: benchmarks/iris-test-data
IRIS_TEST_DATA_VERSION: "2.19"
# Lets us manually bump the cache to rebuild
ENV_CACHE_BUILD: "0"
ENV_CACHE_BUILD: "1"
TEST_DATA_CACHE_BUILD: "2"

steps:
Expand All @@ -37,16 +37,16 @@ jobs:
with:
fetch-depth: 0

- name: Install ASV & Nox
- name: Install ASV & tox
run: |
pip install asv nox
pip install asv tox

- name: Cache environment directories
id: cache-env-dir
uses: actions/cache@v3
with:
path: |
.nox
.tox
benchmarks/.asv/env
$CONDA/pkgs
key: ${{ runner.os }}-${{ hashFiles('requirements/') }}-${{ env.ENV_CACHE_BUILD }}
Expand Down
28 changes: 13 additions & 15 deletions .github/workflows/ci-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,18 @@ jobs:
fail-fast: false
matrix:
os: ["ubuntu-latest"]
python-version: ["3.11"]
session: ["doctest", "gallery", "linkcheck"]
python-version: ["311"]
session: ["docs-tests", "docs-linkcheck", "gallery_tests"]
include:
- os: "ubuntu-latest"
python-version: "3.11"
python-version: "311"
session: "tests"
coverage: "--coverage"
posargs: "--cov=lib/iris --cov-report=xml"
- os: "ubuntu-latest"
python-version: "3.10"
python-version: "310"
session: "tests"
- os: "ubuntu-latest"
python-version: "3.9"
python-version: "39"
session: "tests"

env:
Expand All @@ -63,7 +63,7 @@ jobs:
CACHE_WEEKS: 2
run: |
echo "CACHE_PERIOD=$(date +%Y).$(expr $(date +%U) / ${CACHE_WEEKS})" >> ${GITHUB_ENV}
echo "LOCK_FILE=requirements/locks/py$(echo ${{ matrix.python-version }} | tr -d '.')-linux-64.lock" >> ${GITHUB_ENV}
echo "LOCK_FILE=requirements/locks/py$(echo ${{ matrix.python-version }})-linux-64.lock" >> ${GITHUB_ENV}

- name: "data cache"
uses: ./.github/workflows/composite/iris-data-cache
Expand Down Expand Up @@ -91,10 +91,10 @@ jobs:
- name: "conda environment cache"
uses: ./.github/workflows/composite/conda-env-cache
with:
cache_build: 0
cache_build: 1
cache_period: ${{ env.CACHE_PERIOD }}
env_name: ${{ env.ENV_NAME }}
install_packages: "cartopy nox pip"
install_packages: "cartopy tox'<4'"
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel a little uncomfortable that this remains necessary over 6 months later (see tox-dev/tox-conda#163).

tox-conda looks in need of more maintainers, are we comfortable being dependent on it? @lbdreyer @bjlittle

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need Nox OR Tox anymore, now that we use lock files?!

Having thought more, that was ill-informed, since Tox still offers neat inheritance tricks. But if we stick with tox-conda, we need to be comfortable sticking with the pin as well. Of course being pinned kinda undermines the idea of adopting something more popular.

Thanks @ESadek-MO and @HGWright for conversation on this topic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also leaves us to potential security vulnerabilities:

tox-dev/tox#2524 (comment)


- name: "conda info"
run: |
Expand All @@ -108,8 +108,8 @@ jobs:
cache_period: ${{ env.CACHE_PERIOD }}
env_name: ${{ env.ENV_NAME }}

- name: "nox cache"
uses: ./.github/workflows/composite/nox-cache
- name: "tox cache"
uses: ./.github/workflows/composite/tox-cache
with:
cache_build: 2
env_name: ${{ env.ENV_NAME }}
Expand All @@ -134,11 +134,9 @@ jobs:
cat ${MPL_RC}

- name: "iris ${{ matrix.session }}"
env:
PY_VER: ${{ matrix.python-version }}
run: |
nox --session ${{ matrix.session }} -- --verbose ${{ matrix.coverage }}
tox -e py${{ matrix.python-version }}-${{ matrix.session }} -- ${{ matrix.posargs }}

- name: Upload coverage report
uses: codecov/codecov-action@v3
if: ${{ matrix.coverage }}
if: contains(${{ matrix.posargs }}, "--cov=lib/iris --cov-report=xml")
18 changes: 8 additions & 10 deletions .github/workflows/ci-wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ["3.9", "3.10", "3.11"]
python-version: ["39", "310", "311"]
session: ["wheel"]
env:
ENV_NAME: "ci-wheels"
Expand All @@ -72,7 +72,7 @@ jobs:
CACHE_WEEKS: 2
run: |
echo "CACHE_PERIOD=$(date +%Y).$(expr $(date +%U) / ${CACHE_WEEKS})" >> ${GITHUB_ENV}
echo "LOCK_FILE=requirements/locks/py$(echo ${{ matrix.python-version }} | tr -d '.')-linux-64.lock" >> ${GITHUB_ENV}
echo "LOCK_FILE=requirements/locks/py$(echo ${{ matrix.python-version }})-linux-64.lock" >> ${GITHUB_ENV}

- name: "conda package cache"
uses: ./.github/workflows/composite/conda-pkg-cache
Expand All @@ -93,23 +93,21 @@ jobs:
- name: "conda environment cache"
uses: ./.github/workflows/composite/conda-env-cache
with:
cache_build: 0
cache_build: 1
cache_period: ${{ env.CACHE_PERIOD }}
env_name: ${{ env.ENV_NAME }}
install_packages: "nox pip"
install_packages: "tox'<4' pip"

- name: "nox cache"
uses: ./.github/workflows/composite/nox-cache
- name: "tox cache"
uses: ./.github/workflows/composite/tox-cache
with:
cache_build: 1
env_name: ${{ env.ENV_NAME }}
lock_file: ${{ env.LOCK_FILE }}

- name: "nox install and test wheel"
env:
PY_VER: ${{ matrix.python-version }}
- name: "tox install and test wheel"
run: |
nox --session ${{ matrix.session }} -- --verbose
tox -e py${{ matrix.python-version }}-${{ matrix.session }}

show-artifacts:
needs: build
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
name: "nox cache"
description: "cache the nox test environments"
name: "tox cache"
description: "cache the tox test environments"

inputs:
cache_build:
description: "nox cache build number"
description: "tox cache build number"
required: false
default: "0"
env_name:
Expand All @@ -18,5 +18,5 @@ runs:
steps:
- uses: actions/cache@v3
with:
path: ${{ github.workspace }}/.nox
key: ${{ runner.os }}-nox-${{ inputs.env_name }}-s${{ matrix.session }}-py${{ matrix.python-version }}-b${{ inputs.cache_build }}-${{ hashFiles(inputs.lock_file) }}
path: ${{ github.workspace }}/.tox
key: ${{ runner.os }}-tox-${{ inputs.env_name }}-s${{ matrix.session }}-py${{ matrix.python-version }}-b${{ inputs.cache_build }}-${{ hashFiles(inputs.lock_file) }}
2 changes: 1 addition & 1 deletion MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ exclude codecov.yml
include COPYING
include COPYING.LESSER
exclude Makefile
exclude noxfile.py
exclude tox.ini

# files required to build iris.std_names module
include etc/cf-standard-name-table.xml
Expand Down
7 changes: 3 additions & 4 deletions benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ by the PR. (This run is managed by
[the aforementioned GitHub Action](../.github/workflows/benchmark.yml)).

`asv ...` commands must be run from this directory. You will need to have ASV
installed, as well as Nox (see
installed, as well as tox (see
[Benchmark environments](#benchmark-environments)).

The benchmark runner ([bm_runner.py](./bm_runner.py)) provides conveniences for
Expand Down Expand Up @@ -107,13 +107,12 @@ suites for the UK Met Office NG-VAT project.
## Benchmark environments

We have disabled ASV's standard environment management, instead using an
environment built using the same Nox scripts as Iris' test environments. This
is done using ASV's plugin architecture - see
environment built using tox. This is done using ASV's plugin architecture - see
[asv_delegated_conda.py](asv_delegated_conda.py) and the extra config items in
[asv.conf.json](asv.conf.json).

(ASV is written to control the environment(s) that benchmarks are run in -
minimising external factors and also allowing it to compare between a matrix
of dependencies (each in a separate environment). We have chosen to sacrifice
these features in favour of testing each commit with its intended dependencies,
controlled by Nox + lock-files).
controlled by tox + lock-files).
4 changes: 2 additions & 2 deletions benchmarks/asv.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
// * No build-time environment variables.
// * Is run in the same environment as the ASV install itself.
"delegated_env_commands": [
"PY_VER=3.11 nox --envdir={conf_dir}/.asv/env/nox01 --session=tests --install-only --no-error-on-external-run --verbose"
"tox -c {conf_dir}/.. --devenv={conf_dir}/.asv/env/tox01/bm_env -e py311 --verbose"
],
// The parent directory of the above environment.
// The most recently modified environment in the directory will be used.
"delegated_env_parent": "{conf_dir}/.asv/env/nox01"
"delegated_env_parent": "{conf_dir}/.asv/env/tox01"
}
24 changes: 13 additions & 11 deletions benchmarks/bm_runner.py
trexfeathers marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -62,27 +62,29 @@ def _prep_data_gen_env() -> None:
"""

root_dir = BENCHMARKS_DIR.parent
python_version = "3.11"
python_version = "311"
data_gen_var = "DATA_GEN_PYTHON"
if data_gen_var in environ:
print("Using existing data generation environment.")
else:
print("Setting up the data generation environment ...")
# Get Nox to build an environment for the `tests` session, but don't
# run the session. Will re-use a cached environment if appropriate.
# Get tox to build an environment. It will re-use a cached environment
# if appropriate.
_subprocess_run_print(
[
"nox",
f"--noxfile={root_dir / 'noxfile.py'}",
"--session=tests",
"--install-only",
f"--python={python_version}",
"tox",
"-c",
str(root_dir),
"-e",
f"py{python_version}",
]
)
# Find the environment built above, set it to be the data generation
# environment.
data_gen_python = next(
(root_dir / ".nox").rglob(f"tests*/bin/python{python_version}")
(root_dir / ".tox").rglob(
f"py{python_version}/bin/python{python_version[:1]}.{python_version[1:]}"
)
).resolve()
environ[data_gen_var] = str(data_gen_python)

Expand Down Expand Up @@ -112,7 +114,7 @@ def _prep_data_gen_env() -> None:

def _setup_common() -> None:
_check_requirements("asv")
_check_requirements("nox")
_check_requirements("tox")

_prep_data_gen_env()

Expand Down Expand Up @@ -154,7 +156,7 @@ def _asv_compare(*commits: str, overnight_mode: bool = False) -> None:


class _SubParserGenerator(ABC):
"""Convenience for holding all the necessary argparse info in 1 place."""
"""Convenience for holding all the necessary argparse info in one place."""

name: str = NotImplemented
description: str = NotImplemented
Expand Down
1 change: 1 addition & 0 deletions docs/src/common_links.inc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
.. _sphinx: https://www.sphinx-doc.org/en/master/
.. _sphinx-apidoc: https://github.com/sphinx-contrib/apidoc
.. _test-iris-imagehash: https://github.com/SciTools/test-iris-imagehash
.. _tox: https://tox.readthedocs.io/en/latest/
.. _using git: https://docs.github.com/en/github/using-git
.. _requirements: https://github.com/SciTools/iris/tree/main/requirements
.. _CF-UGRID: https://ugrid-conventions.github.io/ugrid-conventions/
Expand Down
1 change: 1 addition & 0 deletions docs/src/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ def _dotv(version):
"http://www.nationalarchives.gov.uk/doc/open-government-licence",
"https://www.metoffice.gov.uk/",
"https://biggus.readthedocs.io/",
"https://stickler-ci.com/",
]

# list of sources to exclude from the build.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ is merged. Before submitting a pull request please consider the following:
:ref:`code_formatting`.

#. **Check all new dependencies added to the** `requirements`_ **yaml
files.** If dependencies have been added then new nox testing lockfiles
files.** If dependencies have been added then new testing lockfiles
should be generated too, see :ref:`gha_test_env`.

#. **Check the source documentation been updated to explain all new or changed
Expand Down
Loading