Trade 187 #93
Trade 187 #93
Conversation
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
The notebook mostly looks great! I had a couple of comments about test data mainly and reuse of common functionality (I think that was added after you initially opened this PR).
The other thing that this PR would need is the plumbing to hook this up. There are a couple of places that would need updating:
- Adding a new rule the the Makefile that builds images locally (and updating the
imagesrule to include the newsambah-imagerule). - Adding "sambah" to the run_notebooks.sh script. (Thinking about it, this should be made more programmatic in the future)
- Adding "sambah" to the script that is run in Bamboo. (Once TRT-532 is done, this script hopefully goes away, but we're not there, yet)
- Adding this suite to the matrix of images to be built in a GitHub workflow that builds all images.
| "req_infos = {\n", | ||
| " 'UAT': {\n", | ||
| " # TEMPO NO2 tropospheric, stratospheric, and total columns V03\n", | ||
| " 'collection': Collection(id='C1262899916-LARC_CLOUD'),\n", |
There was a problem hiding this comment.
I tried running this notebook and it couldn't find this collection. Maybe it's behind an ACL?
| "\n", | ||
| "compare_results_to_reference_file(temp_var_bbox_filename)\n", | ||
| "compare_results_to_reference_file(temp_var_bbox_filename, 'product')\n", | ||
| "# compare_results_to_reference_file(temp_var_bbox_filename, 'geolocation')\n", |
There was a problem hiding this comment.
Nit: Could this commented out line be removed?
| "), 'Unsuccessful SAMBAH temporal, variable, bounding box request.'\n", | ||
| "\n", | ||
| "\n", | ||
| "compare_results_to_reference_file(temp_var_bbox_filename)\n", |
There was a problem hiding this comment.
Could this be simplified by using datatree? (Albeit from the old location prior to the next xarray release)
There was a problem hiding this comment.
@flamingbear did some fancy stuff a little while back to place a bunch of common utility functions in a shared_utils directory. Maybe some there's overlap here and this file could be simplified?
| } | ||
| }, | ||
| "outputs": [], | ||
| "source": [ |
There was a problem hiding this comment.
A lot of the other test suites wrap their test cells with an if condition to guard against when there aren't test collections/granules defined for an environment (mainly production).
| " 'https://harmony.earthdata.nasa.gov': Environment.PROD,\n", | ||
| "}\n", | ||
| "\n", | ||
| "data_environment = 'PROD' if harmony_host_url == '' else 'UAT'\n", |
There was a problem hiding this comment.
From this the notebook is going to always use the UAT test data values because of this condition, but that feels a bit wonky. That means that a run in production will trigger errors because it will be trying to find UAT data. Is this because there aren't equivalent production data available, yet?
Description
Adds a regression test notebook for the SAMBAH service chain.
Jira Issue ID
Local Test Steps
PR Acceptance Checklist