Skip to content

Commit

Permalink
ensure notebook-testing script fails when notebooks fail (#1424)
Browse files Browse the repository at this point in the history
The notebook-testing CI job in this repo does not actually cause a loud CI failure if any errors are detected in notebooks. That's because of a `bash` mistake I made in #1407... that PR moved notebook-checking into a function, but didn't add a `set -E` to be sure errors from inside that function were appropriately trapped.

This PR fixes that:

* ensures that notebook failures actually cause CI failures
* fixes 2 typos in `nyc_taxi_years_correlation.ipynb` code
  - *(not caught in #1422 because of this CI script bug)*

Context: #1422 (review)

## Notes for Reviewers

### How I tested this

Originally did not skip any notebooks. Saw the failures in one of the notebooks cause an actual merge-blocking CI failure.

Build link: https://github.com/rapidsai/cuspatial/actions/runs/10199784404/job/28219162698?pr=1424

#

Authors:
  - James Lamb (https://github.com/jameslamb)
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - Mark Harris (https://github.com/harrism)
  - https://github.com/jakirkham

URL: #1424
  • Loading branch information
jameslamb authored Aug 13, 2024
1 parent 314dc7d commit 82f321b
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion ci/test_notebooks.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash
# Copyright (c) 2020-2024, NVIDIA CORPORATION.

set -euo pipefail
set -eEuo pipefail

. /opt/conda/etc/profile.d/conda.sh

Expand Down
4 changes: 2 additions & 2 deletions notebooks/nyc_taxi_years_correlation.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@
"taxi_zone_rings = cuspatial.GeoSeries.from_polygons_xy(\n",
" polygons_xy=taxi_zones.polygons.xy,\n",
" ring_offset=taxi_zones.polygons.ring_offset,\n",
" part_offsets=taxi_zones.polygons.part_offset,\n",
" geometry_offsets=cudf.Series(range(len(taxi_zones.polygons.part_offset)))\n",
" part_offset=taxi_zones.polygons.part_offset,\n",
" geometry_offset=cudf.Series(range(len(taxi_zones.polygons.part_offset)))\n",
")"
]
},
Expand Down

0 comments on commit 82f321b

Please sign in to comment.