From 332951499b43ed76ae3630a990fb6c0d865b659b Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:16:59 +0200 Subject: [PATCH 01/12] Run conda install test on windows too --- .github/workflows/install-conda.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/install-conda.yaml b/.github/workflows/install-conda.yaml index 220b2e90..ea333c6a 100644 --- a/.github/workflows/install-conda.yaml +++ b/.github/workflows/install-conda.yaml @@ -22,9 +22,8 @@ jobs: strategy: fail-fast: false matrix: - # # There is an issue on windows, one for another day - # os: ["ubuntu-latest", "macos-latest", "windows-latest"] - os: ["ubuntu-latest", "macos-latest"] + # There is an issue on windows, one for another day + os: ["ubuntu-latest", "macos-latest", "windows-latest"] python-version: [ "3.9", "3.10", "3.11" ] # Check both the 'library' and the 'application' (i.e. locked package) install-target: ["input4mips-validation", "input4mips-validation-locked"] From fb32de18dd1e5146baeab13dd75a81279294c717 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:17:42 +0200 Subject: [PATCH 02/12] Run CI on windows too --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d6dba5b2..8009f7c4 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -42,7 +42,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ "ubuntu-latest" ] + os: [ "ubuntu-latest", "windows-latest" ] test-python-id: [ "py39", "py310", "py311" ] runs-on: "${{ matrix.os }}" defaults: From 869f8ec08a1745f6aef1d960c7fd7ef09c0f33c8 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:24:46 +0200 Subject: [PATCH 03/12] Run CI on windows and mac OS for one python version --- .github/workflows/ci.yaml | 10 +++++++++- .github/workflows/install-conda.yaml | 1 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8009f7c4..a12652a3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -42,8 +42,16 @@ jobs: strategy: fail-fast: false matrix: - os: [ "ubuntu-latest", "windows-latest" ] + os: [ "ubuntu-latest" ] test-python-id: [ "py39", "py310", "py311" ] + include: + # Check that there's no obvious breaks with MacOS or windows. + # Waste of compute to run all cross-combinations, + # particularly given we target linux machines. + - os: macos-latest + test-python-id: py311 + - os: windows-latest + test-python-id: py311 runs-on: "${{ matrix.os }}" defaults: run: diff --git a/.github/workflows/install-conda.yaml b/.github/workflows/install-conda.yaml index ea333c6a..53e77513 100644 --- a/.github/workflows/install-conda.yaml +++ b/.github/workflows/install-conda.yaml @@ -22,7 +22,6 @@ jobs: strategy: fail-fast: false matrix: - # There is an issue on windows, one for another day os: ["ubuntu-latest", "macos-latest", "windows-latest"] python-version: [ "3.9", "3.10", "3.11" ] # Check both the 'library' and the 'application' (i.e. locked package) From 7b14afa31be9ecd02b7ae6585376d6538d188c36 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:25:34 +0200 Subject: [PATCH 04/12] Add extra debugging step --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a12652a3..2ca5c180 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -67,6 +67,7 @@ jobs: pixi-environments: "test-${{ matrix.test-python-id }}" - name: Run tests run: | + pixi run -e all-dev python -c 'import shutil; print(shutil.which("ncdump"))' pixi run --frozen -e "test-${{ matrix.test-python-id }}" pytest -r a -v src tests --doctest-modules --cov=src --cov-report=term-missing --cov-report=xml pixi run --frozen -e "test-${{ matrix.test-python-id }}" coverage report - name: Upload coverage reports to Codecov From ad68a62b0a81f64d6582b034c4f41d6d3f3d9075 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:27:14 +0200 Subject: [PATCH 05/12] Fix up setup python version in test PyPI install --- .github/workflows/install-pypi.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/install-pypi.yaml b/.github/workflows/install-pypi.yaml index 78aac307..b88af069 100644 --- a/.github/workflows/install-pypi.yaml +++ b/.github/workflows/install-pypi.yaml @@ -51,7 +51,7 @@ jobs: run: sudo apt-get install -y netcdf-bin - name: Set up Python "${{ matrix.python-version }}" id: setup-python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: "${{ matrix.python-version }}" - name: Install From 4900890ab2f51ff960ed38db0ab713a55806d01b Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:28:52 +0200 Subject: [PATCH 06/12] Fix up CI definition --- .github/workflows/ci.yaml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2ca5c180..24e028a2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -44,14 +44,14 @@ jobs: matrix: os: [ "ubuntu-latest" ] test-python-id: [ "py39", "py310", "py311" ] - include: - # Check that there's no obvious breaks with MacOS or windows. - # Waste of compute to run all cross-combinations, - # particularly given we target linux machines. - - os: macos-latest - test-python-id: py311 - - os: windows-latest - test-python-id: py311 + include: + # Check that there's no obvious breaks with MacOS or windows. + # Waste of compute to run all cross-combinations, + # particularly given we target linux machines. + - os: macos-latest + test-python-id: py311 + - os: windows-latest + test-python-id: py311 runs-on: "${{ matrix.os }}" defaults: run: From 1c51ef155590fcdf10a8be24787d073ca1ab2021 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:48:32 +0200 Subject: [PATCH 07/12] Add extra check in test --- tests/integration/cli/test_validate_file.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/cli/test_validate_file.py b/tests/integration/cli/test_validate_file.py index 5de519ad..2a870860 100644 --- a/tests/integration/cli/test_validate_file.py +++ b/tests/integration/cli/test_validate_file.py @@ -74,6 +74,10 @@ def test_validate_written_single_variable_file(tmp_path): ncd["lat_bnds"].setncattr("units", "degrees_north") ncd.close() + # Make sure the file exists as expected + if not written_file.exists(): + raise FileNotFoundError(written_file) + # The written file should now fail validation error_msg = re.escape( "WARN: (7.1): Boundary var lat_bnds should not have attribute units" From fc5998cf6613d99068cb355841c859bb1dbedb4d Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 21:57:03 +0200 Subject: [PATCH 08/12] Add dumb debugging --- src/input4mips_validation/validation/exceptions.py | 3 +++ tests/integration/cli/test_validate_file.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/input4mips_validation/validation/exceptions.py b/src/input4mips_validation/validation/exceptions.py index 53fd4129..61ff574b 100644 --- a/src/input4mips_validation/validation/exceptions.py +++ b/src/input4mips_validation/validation/exceptions.py @@ -41,6 +41,9 @@ def __init__( raise AssertionError(msg) # Not clear how input could be further validated hence noqa + subprocess.check_output( + [ncdump_loc, "--help"] # noqa: S603 + ) file_ncdump_h = subprocess.check_output( [ncdump_loc, "-h", str(filepath)] # noqa: S603 ).decode() diff --git a/tests/integration/cli/test_validate_file.py b/tests/integration/cli/test_validate_file.py index 2a870860..0d0c2a12 100644 --- a/tests/integration/cli/test_validate_file.py +++ b/tests/integration/cli/test_validate_file.py @@ -65,8 +65,12 @@ def test_validate_written_single_variable_file(tmp_path): written_file = input4mips_ds.write(root_data_dir=tmp_path) - # # Make sure that the starting file passes - # validate_file(written_file, cv_source=DEFAULT_TEST_INPUT4MIPS_CV_SOURCE) + # Make sure the file exists as expected + if not written_file.exists(): + raise FileNotFoundError(written_file) + + # Make sure that the starting file passes + validate_file(written_file, cv_source=DEFAULT_TEST_INPUT4MIPS_CV_SOURCE) # Add an attribute that shouldn't be there. # This induces a warning in the CF-checker. @@ -85,6 +89,9 @@ def test_validate_written_single_variable_file(tmp_path): with pytest.raises(InvalidFileError, match=error_msg): validate_file(written_file, cv_source=DEFAULT_TEST_INPUT4MIPS_CV_SOURCE) + # Make sure the file exists as expected + if not written_file.exists(): + raise FileNotFoundError(written_file) # The file should pass if we set the right flag validate_file( written_file, From cede7e91a27716156b2509afeda4f6122a57933c Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 22:03:45 +0200 Subject: [PATCH 09/12] Next attempt --- src/input4mips_validation/validation/exceptions.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/input4mips_validation/validation/exceptions.py b/src/input4mips_validation/validation/exceptions.py index 61ff574b..995dac29 100644 --- a/src/input4mips_validation/validation/exceptions.py +++ b/src/input4mips_validation/validation/exceptions.py @@ -34,6 +34,9 @@ def __init__( and the error which was caught while validating the file. """ + if isinstance(filepath, str): + filepath = Path(filepath) + # Not clear how input could be further validated hence noqa ncdump_loc = shutil.which("ncdump") if ncdump_loc is None: @@ -41,11 +44,10 @@ def __init__( raise AssertionError(msg) # Not clear how input could be further validated hence noqa - subprocess.check_output( - [ncdump_loc, "--help"] # noqa: S603 - ) + print(filepath) + print(str(filepath)) file_ncdump_h = subprocess.check_output( - [ncdump_loc, "-h", str(filepath)] # noqa: S603 + [ncdump_loc, "-h", str(filepath.absolute())] # noqa: S603 ).decode() error_msgs: list[str] = [] From 94acdd2a41d2b77eaf0be0cbc94ddacf8d897f7f Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 16 Aug 2024 22:11:01 +0200 Subject: [PATCH 10/12] Tidy up --- .github/workflows/ci.yaml | 1 - src/input4mips_validation/validation/exceptions.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 24e028a2..a3776ac2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -67,7 +67,6 @@ jobs: pixi-environments: "test-${{ matrix.test-python-id }}" - name: Run tests run: | - pixi run -e all-dev python -c 'import shutil; print(shutil.which("ncdump"))' pixi run --frozen -e "test-${{ matrix.test-python-id }}" pytest -r a -v src tests --doctest-modules --cov=src --cov-report=term-missing --cov-report=xml pixi run --frozen -e "test-${{ matrix.test-python-id }}" coverage report - name: Upload coverage reports to Codecov diff --git a/src/input4mips_validation/validation/exceptions.py b/src/input4mips_validation/validation/exceptions.py index 995dac29..05e22334 100644 --- a/src/input4mips_validation/validation/exceptions.py +++ b/src/input4mips_validation/validation/exceptions.py @@ -44,8 +44,6 @@ def __init__( raise AssertionError(msg) # Not clear how input could be further validated hence noqa - print(filepath) - print(str(filepath)) file_ncdump_h = subprocess.check_output( [ncdump_loc, "-h", str(filepath.absolute())] # noqa: S603 ).decode() From a02e3afadab5b2d898d81d5185e20dfb60e6a1cc Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Thu, 22 Aug 2024 14:13:25 +0200 Subject: [PATCH 11/12] Try removing netCDF4 call --- tests/integration/cli/test_validate_file.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/integration/cli/test_validate_file.py b/tests/integration/cli/test_validate_file.py index 0d0c2a12..5e71238b 100644 --- a/tests/integration/cli/test_validate_file.py +++ b/tests/integration/cli/test_validate_file.py @@ -9,7 +9,6 @@ from pathlib import Path from unittest.mock import patch -import netCDF4 import numpy as np import pint import pint_xarray # noqa: F401 # required to activate pint accessor @@ -72,11 +71,11 @@ def test_validate_written_single_variable_file(tmp_path): # Make sure that the starting file passes validate_file(written_file, cv_source=DEFAULT_TEST_INPUT4MIPS_CV_SOURCE) - # Add an attribute that shouldn't be there. - # This induces a warning in the CF-checker. - ncd = netCDF4.Dataset(written_file, "a") - ncd["lat_bnds"].setncattr("units", "degrees_north") - ncd.close() + # # Add an attribute that shouldn't be there. + # # This induces a warning in the CF-checker. + # ncd = netCDF4.Dataset(written_file, "a") + # ncd["lat_bnds"].setncattr("units", "degrees_north") + # ncd.close() # Make sure the file exists as expected if not written_file.exists(): From e404d8f6510c09c89ffa61fefe8814f9a10cb8f9 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Thu, 22 Aug 2024 14:16:20 +0200 Subject: [PATCH 12/12] Revert "Try removing netCDF4 call" This reverts commit a02e3afadab5b2d898d81d5185e20dfb60e6a1cc. --- tests/integration/cli/test_validate_file.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/integration/cli/test_validate_file.py b/tests/integration/cli/test_validate_file.py index 5e71238b..0d0c2a12 100644 --- a/tests/integration/cli/test_validate_file.py +++ b/tests/integration/cli/test_validate_file.py @@ -9,6 +9,7 @@ from pathlib import Path from unittest.mock import patch +import netCDF4 import numpy as np import pint import pint_xarray # noqa: F401 # required to activate pint accessor @@ -71,11 +72,11 @@ def test_validate_written_single_variable_file(tmp_path): # Make sure that the starting file passes validate_file(written_file, cv_source=DEFAULT_TEST_INPUT4MIPS_CV_SOURCE) - # # Add an attribute that shouldn't be there. - # # This induces a warning in the CF-checker. - # ncd = netCDF4.Dataset(written_file, "a") - # ncd["lat_bnds"].setncattr("units", "degrees_north") - # ncd.close() + # Add an attribute that shouldn't be there. + # This induces a warning in the CF-checker. + ncd = netCDF4.Dataset(written_file, "a") + ncd["lat_bnds"].setncattr("units", "degrees_north") + ncd.close() # Make sure the file exists as expected if not written_file.exists():