From 1a50424da910a2a3cfd724daa2c0be29f36eb4d5 Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Sat, 12 Apr 2025 11:22:06 -0400 Subject: [PATCH 1/7] Cell magics are run ina subprocess and the errors are captured, therefore failing tutorials were showing as passing. Signed-off-by: Israel Martinez --- docs/tutorials/run_tutorials.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/tutorials/run_tutorials.py b/docs/tutorials/run_tutorials.py index feb12e8f..39165759 100755 --- a/docs/tutorials/run_tutorials.py +++ b/docs/tutorials/run_tutorials.py @@ -15,7 +15,7 @@ from pathlib import Path import nbformat -from nbconvert.preprocessors import ExecutePreprocessor +from nbconvert.preprocessors import ExecutePreprocessor, RegexRemovePreprocessor from nbconvert import HTMLExporter from nbconvert.writers import FilesWriter @@ -221,6 +221,14 @@ def run_tutorial(tutorial): with (open(nb_path) as nb_file): nb = nbformat.read(nb_file, as_version=nbformat.NO_CONVERT) + # Remove magic, which can make a failing notebook look + # like it succeeded. + for cell in nb.cells: + if cell.cell_type == 'code': + source = cell.source.strip("\n").lstrip() + if len(source) >= 1 and source[0] == "%": + cell.source = cell.source.replace("%", "#[magic commented out by run_tutorials.py]%") + logger.info(f"Executing notebook {source_nb_path}...") start_time = timeit.default_timer() ep = ExecutePreprocessor(timeout=config['globals:timeout'], kernel_name=config['globals:kernel']) From 6559f842c8531510f22a9ba6a198f6119dcd6ee0 Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Sat, 12 Apr 2025 18:25:06 -0400 Subject: [PATCH 2/7] Remove only magic cell. I think magic lines are not problematic Signed-off-by: Israel Martinez --- docs/tutorials/run_tutorials.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/tutorials/run_tutorials.py b/docs/tutorials/run_tutorials.py index 39165759..747b8957 100755 --- a/docs/tutorials/run_tutorials.py +++ b/docs/tutorials/run_tutorials.py @@ -226,8 +226,8 @@ def run_tutorial(tutorial): for cell in nb.cells: if cell.cell_type == 'code': source = cell.source.strip("\n").lstrip() - if len(source) >= 1 and source[0] == "%": - cell.source = cell.source.replace("%", "#[magic commented out by run_tutorials.py]%") + if len(source) >= 2 and source[:2] == "%%": + cell.source = cell.source.replace("%%", "#[magic commented out by run_tutorials.py]%%") logger.info(f"Executing notebook {source_nb_path}...") start_time = timeit.default_timer() From 55d9d7eca6a26c5c259b95ecb2009f8debdff7aa Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Sat, 12 Apr 2025 18:25:49 -0400 Subject: [PATCH 3/7] Add test for magic cells. Also serves as a negative control Signed-off-by: Israel Martinez --- docs/tutorials/run_tutorials.yml | 2 + .../this_test_must_fail_remove_magic.ipynb | 73 +++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 docs/tutorials/test/this_test_must_fail_remove_magic.ipynb diff --git a/docs/tutorials/run_tutorials.yml b/docs/tutorials/run_tutorials.yml index b6433478..1fabbe4a 100644 --- a/docs/tutorials/run_tutorials.yml +++ b/docs/tutorials/run_tutorials.yml @@ -31,6 +31,8 @@ tutorials: unzip: True # Optional. False by default #unzip_output: # Optional, if the unzipped file name is different from just removing the .zip or .gz + test_magic_test_must_fail: + notebook: test/this_test_must_fail_remove_magic.ipynb dataIO: notebook: DataIO/DataIO_example.ipynb diff --git a/docs/tutorials/test/this_test_must_fail_remove_magic.ipynb b/docs/tutorials/test/this_test_must_fail_remove_magic.ipynb new file mode 100644 index 00000000..2f4277f7 --- /dev/null +++ b/docs/tutorials/test/this_test_must_fail_remove_magic.ipynb @@ -0,0 +1,73 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "id": "b82642d2-a68f-41d2-8a63-3fee53880c71", + "metadata": {}, + "source": [ + "# Test magic removal" + ] + }, + { + "cell_type": "markdown", + "id": "b42b3af6-b4e6-4809-bbcb-8917202d5c44", + "metadata": {}, + "source": [ + "Magic cells are run in a subprocess, which catches exceptions and makes it look like the notebook succeeded " + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "7e9ae042-38d3-4f51-9fd9-0f630bdc1e48", + "metadata": {}, + "outputs": [], + "source": [ + "%%time\n", + "# This should fail since \"five\" has not been defined.\n", + "5*five" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "eb67f11c-b8cc-42ef-ac81-ac01fd6a88da", + "metadata": {}, + "outputs": [], + "source": [ + "%%time\n", + "# It shouldn't make it to this cell\n", + "5*5" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "ac335794-7781-410e-9b60-d0dfe2ef6ad3", + "metadata": {}, + "outputs": [], + "source": [] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python [conda env:cosipy]", + "language": "python", + "name": "conda-env-cosipy-py" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.10.16" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} From 0e408cc57e3b8f2d763083b1d413e4164d94a6de Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Sat, 12 Apr 2025 18:29:02 -0400 Subject: [PATCH 4/7] Change test name Signed-off-by: Israel Martinez --- docs/tutorials/run_tutorials.yml | 4 ++-- ...ust_fail_remove_magic.ipynb => test_must_fail_magic.ipynb} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename docs/tutorials/test/{this_test_must_fail_remove_magic.ipynb => test_must_fail_magic.ipynb} (100%) diff --git a/docs/tutorials/run_tutorials.yml b/docs/tutorials/run_tutorials.yml index 1fabbe4a..214aea9c 100644 --- a/docs/tutorials/run_tutorials.yml +++ b/docs/tutorials/run_tutorials.yml @@ -31,8 +31,8 @@ tutorials: unzip: True # Optional. False by default #unzip_output: # Optional, if the unzipped file name is different from just removing the .zip or .gz - test_magic_test_must_fail: - notebook: test/this_test_must_fail_remove_magic.ipynb + test_must_fail_magic: + notebook: test/test_must_fail_magic.ipynb dataIO: notebook: DataIO/DataIO_example.ipynb diff --git a/docs/tutorials/test/this_test_must_fail_remove_magic.ipynb b/docs/tutorials/test/test_must_fail_magic.ipynb similarity index 100% rename from docs/tutorials/test/this_test_must_fail_remove_magic.ipynb rename to docs/tutorials/test/test_must_fail_magic.ipynb From 83d04afb0e4e003afd7ac8d84ca2f4a81f82b526 Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Sat, 12 Apr 2025 18:34:40 -0400 Subject: [PATCH 5/7] Fail successfully = Green! Signed-off-by: Israel Martinez --- docs/tutorials/run_tutorials.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/tutorials/run_tutorials.py b/docs/tutorials/run_tutorials.py index 747b8957..0a71da23 100755 --- a/docs/tutorials/run_tutorials.py +++ b/docs/tutorials/run_tutorials.py @@ -278,7 +278,11 @@ def run_tutorial(tutorial): if succeeded: logger.info(colorama.Fore.GREEN + "SUCCEEDED " + colorama.Style.RESET_ALL + f"({elapsed:.1f} s) {tutorial}") else: - logger.info(colorama.Fore.RED + "FAILED " + colorama.Style.RESET_ALL + f"({elapsed:.1f} s) {tutorial}") + color = colorama.Fore.RED + if "test_must_fail" in tutorial: + # Failed succesfully! + color = colorama.Fore.GREEN + logger.info(color + "FAILED " + colorama.Style.RESET_ALL + f"({elapsed:.1f} s) {tutorial}") # Overall summary log logger.info(f"cosipy version: {cosipy.__version__}") @@ -291,7 +295,11 @@ def run_tutorial(tutorial): if succeeded: logger.info(colorama.Fore.GREEN + "SUCCEEDED " + colorama.Style.RESET_ALL + f"({elapsed:.1f} s) {tutorial}") else: - logger.info(colorama.Fore.RED + "FAILED " + colorama.Style.RESET_ALL + f"({elapsed:.1f} s) {tutorial}") + color = colorama.Fore.RED + if "test_must_fail" in tutorial: + # Failed succesfully! + color = colorama.Fore.GREEN + logger.info(color + "FAILED " + colorama.Style.RESET_ALL + f"({elapsed:.1f} s) {tutorial}") if __name__ == "__main__": From beafef096059b585c41ec87678d4e865bdba25f1 Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Mon, 14 Apr 2025 21:07:47 -0400 Subject: [PATCH 6/7] Fix file checksum in ts map tutorial Signed-off-by: Israel Martinez --- docs/tutorials/ts_map/Parallel_TS_map_computation.ipynb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tutorials/ts_map/Parallel_TS_map_computation.ipynb b/docs/tutorials/ts_map/Parallel_TS_map_computation.ipynb index 4a3c31a2..fb29de59 100644 --- a/docs/tutorials/ts_map/Parallel_TS_map_computation.ipynb +++ b/docs/tutorials/ts_map/Parallel_TS_map_computation.ipynb @@ -704,7 +704,7 @@ "\n", "GRB_signal_path = data_dir/\"grb_binned_data.hdf5\"\n", "# download GRB signal file ~76.90 KB\n", - "fetch_wasabi_file(\"COSI-SMEX/cosipy_tutorials/grb_spectral_fit_local_frame/grb_binned_data.hdf5\", GRB_signal_path, checksum = 'fce391a4b45624b25552c7d111945f60')\n", + "fetch_wasabi_file(\"COSI-SMEX/cosipy_tutorials/grb_spectral_fit_local_frame/grb_binned_data.hdf5\", GRB_signal_path, checksum = 'fcf7022369b6fb378d67b780fc4b5db8')\n", "\n", "background_path = data_dir/\"bkg_binned_data_local.hdf5\"\n", "# download background file ~255.97 MB\n", From 122d4ad06f978023ef93e19d05ca3756933e3b26 Mon Sep 17 00:00:00 2001 From: Israel Martinez Date: Mon, 14 Apr 2025 21:25:52 -0400 Subject: [PATCH 7/7] Catch nb failure exception to allow notebook to still be saved and clean log file handler Signed-off-by: Israel Martinez --- docs/tutorials/run_tutorials.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/tutorials/run_tutorials.py b/docs/tutorials/run_tutorials.py index 0a71da23..dff898e0 100755 --- a/docs/tutorials/run_tutorials.py +++ b/docs/tutorials/run_tutorials.py @@ -3,6 +3,8 @@ import logging import traceback +from nbclient.exceptions import CellExecutionError + logging.basicConfig(format='%(asctime)s - %(levelname)s - %(filename)s:%(lineno)d - %(message)s', datefmt='%Y-%m-%d %H:%M:%S', level=logging.INFO) @@ -213,6 +215,7 @@ def run_tutorial(tutorial): os.symlink(local_copy, wdir/local_copy.name) # Run + failed = False if not args.dry: for notebook in notebooks: source_nb_path = config.absolute_path(notebook) @@ -232,10 +235,22 @@ def run_tutorial(tutorial): logger.info(f"Executing notebook {source_nb_path}...") start_time = timeit.default_timer() ep = ExecutePreprocessor(timeout=config['globals:timeout'], kernel_name=config['globals:kernel']) - ep_out = ep.preprocess(nb, {'metadata': {'path': str(wdir)}}) + + try: + ep_out = ep.preprocess(nb, {'metadata': {'path': str(wdir)}}) + except CellExecutionError as e: + # Will re-raise after output and cleaning + cell_exception = e + failed = True + elapsed = timeit.default_timer() - start_time - logger.info(f"Notebook {source_nb_path} took {elapsed} seconds to finish.") + if failed: + logger.error(f"Notebook {source_nb_path} failed after {elapsed} seconds") + else: + logger.info(f"Notebook {source_nb_path} took {elapsed} seconds to finish.") + + # Save output nb_exec_path = nb_path.with_name(nb_path.stem + "_executed" + nb_path.suffix) with open(nb_exec_path, 'w', encoding='utf-8') as exec_nb_file: nbformat.write(nb, exec_nb_file) @@ -250,6 +265,10 @@ def run_tutorial(tutorial): # Remove file logger logger.removeHandler(file_handler) + # Re-raise if failed + if failed: + raise cell_exception + # Loop through each tutorial summary = {} for tutorial in tutorials: