From ddfdd77e57d8b53daab68bcc9edf413badd3bf89 Mon Sep 17 00:00:00 2001 From: Matt Savoie Date: Fri, 11 Oct 2024 17:56:38 -0600 Subject: [PATCH] DAS-2267: Switch trajectory-subsetter over to use the shared_utilies --- .github/workflows/build-all-images.yml | 1 + test/Makefile | 2 +- .../NSIDC-ICESAT2_Regression.ipynb | 10 +-- test/shared_utils/README.md | 16 +++- test/shared_utils/compare.py | 31 +------ .../TrajectorySubsetter_Regression.ipynb | 25 +++--- test/trajectory-subsetter/environment.yaml | 15 ++-- test/trajectory-subsetter/local_utilities.py | 19 +++++ test/trajectory-subsetter/utilities.py | 82 ------------------- test/trajectory-subsetter/version.txt | 2 +- 10 files changed, 68 insertions(+), 135 deletions(-) create mode 100644 test/trajectory-subsetter/local_utilities.py delete mode 100644 test/trajectory-subsetter/utilities.py diff --git a/.github/workflows/build-all-images.yml b/.github/workflows/build-all-images.yml index b9e56386..c6c6a184 100644 --- a/.github/workflows/build-all-images.yml +++ b/.github/workflows/build-all-images.yml @@ -43,6 +43,7 @@ jobs: - image: "trajectory-subsetter" notebook: "TrajectorySubsetter_Regression.ipynb" + shared-utils: "true" - image: "variable-subsetter" notebook: "VariableSubsetter_Regression.ipynb" diff --git a/test/Makefile b/test/Makefile index 503c69f5..8ebc221f 100644 --- a/test/Makefile +++ b/test/Makefile @@ -36,7 +36,7 @@ swath-projector-image: Dockerfile swath-projector/environment.yaml trajectory-subsetter-image: Dockerfile trajectory-subsetter/environment.yaml docker build -t ghcr.io/nasa/regression-tests-trajectory-subsetter:latest -f ./Dockerfile \ - --build-arg notebook=TrajectorySubsetter_Regression.ipynb --build-arg sub_dir=trajectory-subsetter . + --build-arg notebook=TrajectorySubsetter_Regression.ipynb --build-arg sub_dir=trajectory-subsetter --build-arg shared_utils=true . variable-subsetter-image: Dockerfile variable-subsetter/environment.yaml docker build -t ghcr.io/nasa/regression-tests-variable-subsetter:latest -f ./Dockerfile \ diff --git a/test/nsidc-icesat2/NSIDC-ICESAT2_Regression.ipynb b/test/nsidc-icesat2/NSIDC-ICESAT2_Regression.ipynb index 35f5c9f0..d9d06989 100644 --- a/test/nsidc-icesat2/NSIDC-ICESAT2_Regression.ipynb +++ b/test/nsidc-icesat2/NSIDC-ICESAT2_Regression.ipynb @@ -122,7 +122,7 @@ "\n", "sys.path.append('../shared_utils')\n", "from utilities import print_success, submit_and_download\n", - "from compare import compare_results_to_reference_file_new" + "from compare import compare_results_to_reference_file" ] }, { @@ -381,7 +381,7 @@ " assert exists(\n", " test_output\n", " ), 'Unsuccessful Harmony Request: {shortname}: {test_name}'\n", - " compare_results_to_reference_file_new(\n", + " compare_results_to_reference_file(\n", " test_output, test_reference, identical=False\n", " )\n", " print_success(f'{shortname} {test_name} test request.')\n", @@ -435,7 +435,7 @@ " assert exists(\n", " test_output\n", " ), 'Unsuccessful Harmony Request: {shortname}: {test_name}'\n", - " compare_results_to_reference_file_new(\n", + " compare_results_to_reference_file(\n", " test_output,\n", " test_reference,\n", " identical=False,\n", @@ -492,7 +492,7 @@ " assert exists(\n", " test_output\n", " ), 'Unsuccessful Harmony Request: {shortname}: {test_name}'\n", - " compare_results_to_reference_file_new(\n", + " compare_results_to_reference_file(\n", " test_output, test_reference, identical=False, coordinates_to_fix=[]\n", " )\n", " print_success(f'{shortname} {test_name} test request.')\n", @@ -521,7 +521,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.10" + "version": "3.11.5" } }, "nbformat": 4, diff --git a/test/shared_utils/README.md b/test/shared_utils/README.md index a9bfbe69..0d8a14f6 100644 --- a/test/shared_utils/README.md +++ b/test/shared_utils/README.md @@ -1,5 +1,7 @@ ## This directory contains common utility functions that can be shared across regression tests. +## Include the build arg on the Makefile for your tests + This directory can be included in your test suite by adding a build-arg to the docker build command in the Makefile. ```sh @@ -8,7 +10,19 @@ nsidc-icesat2-image: Dockerfile nsidc-icesat2/environment.yaml --build-arg notebook=NSIDC-ICESAT2_Regression.ipynb --build-arg sub_dir=nsidc-icesat2 --build-arg shared_utils=true . ``` -Doing this will cause this directory and all its files to be included at `/workdir/shared_utils` in your container. +Doing this will cause this directory and all its files to be included at `/workdir/shared_utils` in your container when you are working locally. + +## Update github workflows to include the build arg for your tests. + +To include the shared_utils directory on the regression image built by GitHub you add a `shared_utils` key to the service matrix under your service like was done for the trajectory subsetter in the `.github/workflows/build-all-images.yml` file. + +```yml + - + image: "trajectory-subsetter" + notebook: "TrajectorySubsetter_Regression.ipynb" + shared-utils: "true" + +``` ## Include the necessary python packages in your test's pip_requirements.txt diff --git a/test/shared_utils/compare.py b/test/shared_utils/compare.py index bca7ffe5..b4a69a50 100644 --- a/test/shared_utils/compare.py +++ b/test/shared_utils/compare.py @@ -7,36 +7,13 @@ from itertools import count -try: - from xarray.backends.api import open_groups - from xarray.core.datatree import DataTree - from xarray import Dataset -except Exception: - # only used by Trajectory Subsetter tests. - # TODO: remove and make Trajectory Subsetter use above - from datatree import open_datatree +from xarray.backends.api import open_groups +from xarray.core.datatree import DataTree +from xarray import Dataset -def compare_results_to_reference_file( - results_file_name: str, reference_file_name: str -) -> None: - """Use `DataTree` functionality to compare data values, variables, - coordinates, metadata, and all their corresponding attributes of - downloaded results to a reference file. - - """ - reference_data = open_datatree(reference_file_name) - results_data = open_datatree(results_file_name) - assert results_data.identical(reference_data), ( - 'Output and reference files ' 'do not match.' - ) - - reference_data = None - results_data = None - - -def compare_results_to_reference_file_new( +def compare_results_to_reference_file( results_file_name: str, reference_file_name: str, identical: bool = True, diff --git a/test/trajectory-subsetter/TrajectorySubsetter_Regression.ipynb b/test/trajectory-subsetter/TrajectorySubsetter_Regression.ipynb index bc0951b6..4262aa78 100644 --- a/test/trajectory-subsetter/TrajectorySubsetter_Regression.ipynb +++ b/test/trajectory-subsetter/TrajectorySubsetter_Regression.ipynb @@ -37,25 +37,30 @@ { "cell_type": "code", "execution_count": null, - "id": "7ce7f640", + "id": "f4489f6d-50d2-46a3-ad52-1da296237693", "metadata": { - "jupyter": { - "source_hidden": true - }, "tags": [] }, "outputs": [], "source": [ + "import sys\n", + "\n", + "sys.path.append('../shared_utils')\n", + "from utilities import (\n", + " print_success,\n", + " submit_and_download,\n", + ")\n", + "\n", + "from compare import compare_results_to_reference_file\n", + "\n", + "\n", "from datetime import datetime\n", "from os.path import exists\n", "\n", "from harmony import BBox, Client, Collection, Environment, Request\n", "\n", - "from utilities import (\n", - " compare_results_to_reference_file,\n", - " print_success,\n", + "from local_utilities import (\n", " remove_results_files,\n", - " submit_and_download,\n", ")" ] }, @@ -388,7 +393,7 @@ "metadata": { "celltoolbar": "Tags", "kernelspec": { - "display_name": "papermill-trajectory-subsetter", + "display_name": "Python 3 (ipykernel)", "language": "python", "name": "python3" }, @@ -402,7 +407,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.10.12" + "version": "3.11.5" }, "name": "SDS_Regression.ipynb" }, diff --git a/test/trajectory-subsetter/environment.yaml b/test/trajectory-subsetter/environment.yaml index 4e1ffaa1..31234221 100644 --- a/test/trajectory-subsetter/environment.yaml +++ b/test/trajectory-subsetter/environment.yaml @@ -3,12 +3,11 @@ channels: - conda-forge - defaults dependencies: - - python=3.10 - - notebook=6.5.4 - - papermill=2.3.4 - - python-dateutil=2.7.5 - - numpy=1.24.3 - - netCDF4=1.6.3 - - xarray-datatree=0.0.12 + - python=3.11.5 + - notebook=7.2.1 + - papermill=2.6 + - numpy=2.1.2 + - netCDF4=1.7.1 - pip: - - harmony-py + - harmony-py==0.4.15 + - xarray==2024.9.0 diff --git a/test/trajectory-subsetter/local_utilities.py b/test/trajectory-subsetter/local_utilities.py new file mode 100644 index 00000000..0913281d --- /dev/null +++ b/test/trajectory-subsetter/local_utilities.py @@ -0,0 +1,19 @@ +""" A module containing utility functionality used by the Trajectory Subsetter + regression tests. These functions are kept out of the Jupyter notebook to + increase the readability of the regression test suite. + +""" + +from os import listdir, remove + + +def remove_results_files() -> None: + """Remove all NetCDF-4 files downloaded during the Swath Projector + regression tests. + + """ + directory_files = listdir() + + for directory_file in directory_files: + if directory_file.endswith('.h5'): + remove(directory_file) diff --git a/test/trajectory-subsetter/utilities.py b/test/trajectory-subsetter/utilities.py deleted file mode 100644 index d682d97f..00000000 --- a/test/trajectory-subsetter/utilities.py +++ /dev/null @@ -1,82 +0,0 @@ -""" A module containing utility functionality used by the Trajectory Subsetter - regression tests. These functions are kept out of the Jupyter notebook to - increase the readability of the regression test suite. - -""" - -from os import listdir, remove, replace - -from harmony import Client, Request -from harmony.harmony import ProcessingFailedException -from datatree import open_datatree - - -def compare_results_to_reference_file( - results_file_name: str, reference_file_name: str -) -> None: - """Use `DataTree` functionality to compare data values, variables, - coordinates, metadata, and all their corresponding attributes of - downloaded results to a reference file. - - """ - reference_data = open_datatree(reference_file_name) - results_data = open_datatree(results_file_name) - - assert results_data.identical(reference_data), ( - 'Output and reference files ' 'do not match.' - ) - - reference_data = None - results_data = None - - -def submit_and_download( - harmony_client: Client, request: Request, output_file_name: str -): - """Submit a Harmony request via a `harmony-py` client. Wait for the - Harmony job to finish, then download the results to the specified file - path. - - """ - downloaded_filename = None - - try: - job_id = harmony_client.submit(request) - - for filename in [ - file_future.result() - for file_future in harmony_client.download_all(job_id, overwrite=True) - ]: - - print(f'Downloaded: {filename}') - downloaded_filename = filename - - if downloaded_filename is not None: - replace(downloaded_filename, output_file_name) - print(f'Saved output to: {output_file_name}') - - except ProcessingFailedException as exception: - print_error('Harmony request failed to complete successfully.') - raise exception - - -def remove_results_files() -> None: - """Remove all NetCDF-4 files downloaded during the Swath Projector - regression tests. - - """ - directory_files = listdir() - - for directory_file in directory_files: - if directory_file.endswith('.h5'): - remove(directory_file) - - -def print_error(error_string: str) -> str: - """Print an error, with formatting for red text.""" - print(f'\033[91m{error_string}\033[0m') - - -def print_success(success_string: str) -> str: - """Print a success message, with formatting for green text.""" - print(f'\033[92mSuccess: {success_string}\033[0m') diff --git a/test/trajectory-subsetter/version.txt b/test/trajectory-subsetter/version.txt index c946ee61..11808190 100644 --- a/test/trajectory-subsetter/version.txt +++ b/test/trajectory-subsetter/version.txt @@ -1 +1 @@ -0.1.6 +0.1.7