-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New examples for the updated documentation #495
Changes from all commits
73dc0de
b5bf962
a31700b
362a25a
8cb4dd7
b2088a0
a35430e
3ab770e
ee2a158
f688059
e8a9987
faa2c62
9d688df
936fa62
acede13
ee532a7
a1359d4
74bee15
294db24
8fa5d7b
307fb40
52d9723
7a6b068
59f6b97
948ea0e
4432c65
e209d8c
103ff06
1481110
bca3c32
b8ab12f
6fabda8
a6ca0f5
21b7099
ee386f7
1632ba1
99e5126
0b239e2
a11e378
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,11 @@ | ||||||||||||||||||||||||||||||||||||||||||
#!/bin/bash | ||||||||||||||||||||||||||||||||||||||||||
# execute notebooks | ||||||||||||||||||||||||||||||||||||||||||
i=0; | ||||||||||||||||||||||||||||||||||||||||||
for notebook in $(ls notebooks/*.ipynb); do | ||||||||||||||||||||||||||||||||||||||||||
papermill ${notebook} ${notebook%.*}-out.${notebook##*.} -k python3 || i=$((i+1)); | ||||||||||||||||||||||||||||||||||||||||||
done; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
# push error to next level | ||||||||||||||||||||||||||||||||||||||||||
if [ $i -gt 0 ]; then | ||||||||||||||||||||||||||||||||||||||||||
exit 1; | ||||||||||||||||||||||||||||||||||||||||||
fi; | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+8
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error reporting with execution summary The current error handling doesn't provide enough information about the execution results. Apply this diff to improve error reporting: -# push error to next level
-if [ $i -gt 0 ]; then
- exit 1;
-fi;
+# Print execution summary
+total=$(find notebooks -name "*.ipynb" | wc -l)
+successful=$((total - i))
+
+echo "Notebook Execution Summary:"
+echo "-------------------------"
+echo "Total notebooks: $total"
+echo "Successfully executed: $successful"
+echo "Failed executions: $i"
+
+if [ $i -gt 0 ]; then
+ echo "Error: $i notebook(s) failed execution. Check logs directory for details." >&2
+ exit 1
+fi
+
+echo "All notebooks executed successfully!" 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,4 +34,4 @@ jobs: | |
timeout-minutes: 5 | ||
run: > | ||
flux start | ||
papermill notebooks/examples.ipynb examples-out.ipynb -k "python3" | ||
.ci_support/build_notebooks.sh | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Increase the workflow timeout. The current 5-minute timeout may be insufficient for executing all 5 notebooks. Consider increasing the 🔗 Analysis chainConsider increasing the workflow timeout. The workflow is now executing multiple notebooks through the shell script, but still uses a 5-minute timeout. This might be insufficient when running all notebooks sequentially, especially those involving HPC and resource allocation examples. Consider:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Count notebooks and estimate required time
echo "Number of notebooks to be executed:"
fd .ipynb$ notebooks/ | wc -l
Length of output: 535 Script: #!/bin/bash
# Count the number of Jupyter notebooks
echo "Total number of notebooks:"
fd .ipynb$ notebooks/ | wc -l
Length of output: 98 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix fragile file iteration and improve error logging
The current implementation has several issues:
ls
for iteration is fragile and will break with spaces in filenamesApply this diff to make the script more robust:
📝 Committable suggestion
🧰 Tools
🪛 Shellcheck
[error] 4-4: Iterating over ls output is fragile. Use globs.
(SC2045)