FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Clean notebook metadata by JDBetteridge · Pull Request #2639 · devitocodes/devito · GitHub

/ devito Public

Clean notebook metadata - #2639

Merged
FabioLuporini merged 15 commits into
mainfrom
JDBetteridge/fix_online_notebook_rendering
Jul 29, 2025
ConversationCommits (15)ChecksFiles changed
Merged

Clean notebook metadata#2639
FabioLuporini merged 15 commits into
mainfrom
JDBetteridge/fix_online_notebook_rendering

Conversation

JDBetteridge commented Jun 23, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Remove the metadata to allow notebooks to render on Github

review-notebook-app Bot commented Jun 23, 2025

Copy link
Copy Markdown

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov Bot commented Jun 23, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.34%. Comparing base (cd9058b) to head (30b1eb6).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #2639       +/-   ##
===========================================
+ Coverage   78.59%   91.34%   +12.74%     
===========================================
  Files         245      245               
  Lines       49089    49089               
  Branches     4322     4322               
===========================================
+ Hits        38582    44840     +6258     
+ Misses       9714     3531     -6183     
+ Partials      793      718       -75
Flag Coverage Δ
pytest-gpu-aomp-amdgpuX 72.50% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JDBetteridge force-pushed the JDBetteridge/fix_online_notebook_rendering branch from 8a13b99 to bd5d198 Compare July 23, 2025 16:20
JDBetteridge changed the title Try cleaning a single notebook Clean notebook metadata Jul 23, 2025
mloubout reviewed Jul 23, 2025
View reviewed changes

mloubout left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oooof that's quite the diff. Does it really need to. rerun and redo all the figures/.... ?

EdCaunt approved these changes Jul 23, 2025
View reviewed changes

EdCaunt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Notebooks all seem to render correctly now. Would be nice to not stage the images, but it is a major pain to manually unstage them for this many notebooks

ggorman requested a review from Copilot July 24, 2025 08:54

This comment was marked as outdated.

Sign in to view

FabioLuporini reviewed Jul 24, 2025
View reviewed changes

FabioLuporini left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I can live with the edited images as part of this PR

However, some tests are now failing

JDBetteridge commented Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

I'm on it, just a breaking line in a string

mloubout approved these changes Jul 24, 2025
View reviewed changes

mloubout left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

need rebase and gtg

JDBetteridge force-pushed the JDBetteridge/fix_online_notebook_rendering branch from bec9dd7 to 6894256 Compare July 24, 2025 13:40

JDBetteridge commented Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

Please don't hit merge! I've spotted a rendering error (which I'm fixing)

JDBetteridge force-pushed the JDBetteridge/fix_online_notebook_rendering branch from 6894256 to 98a23e0 Compare July 24, 2025 15:58

JDBetteridge commented Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

A few observations that I won't address in this PR:

  • CFD notebook 8 the shallow water equations is unstable if re-run. The solutions all blow up before the final time
  • The CI checks against output that you don't get with a current install (ie: in newer installations the output of numpy floats is different to what is being checked against)
  • ffmpeg isn't installed in the docker container so the notebooks that have videos don't run

ggorman requested a review from Copilot July 25, 2025 11:33
Copilot AI reviewed Jul 25, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull Request Overview

This PR cleans notebook metadata to improve GitHub rendering compatibility. The changes strip out unnecessary metadata while preserving essential notebook functionality.

  • Creates a shell script to automate notebook cleaning process
  • Standardizes cell IDs from UUIDs to sequential numeric format
  • Removes extra metadata fields that interfere with GitHub display

Reviewed Changes

Copilot reviewed 2 out of 35 changed files in this pull request and generated 1 comment.

File Description
scripts/clean_notebooks.sh Adds automated notebook cleaning script with nbconvert and nbstripout
examples/cfd/08_shallow_water_equation.ipynb Replaces UUID cell IDs with sequential numbers and removes extra metadata
Comments suppressed due to low confidence (1)

scripts/clean_notebooks.sh:7

  • The environment variable name PYTEST_VERSION is misleading. This appears to be setting a flag for numpy output formatting rather than specifying a pytest version. Consider renaming to something like NUMPY_LEGACY_PRINT=1 or NUMPY_SCALAR_OUTPUT=1 to better reflect its purpose.
export PYTEST_VERSION=1

Comment thread scripts/clean_notebooks.sh
FabioLuporini merged commit a847052 into main Jul 29, 2025
35 of 36 checks passed
FabioLuporini deleted the JDBetteridge/fix_online_notebook_rendering branch July 29, 2025 12:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

FabioLuporini FabioLuporini left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
mloubout mloubout approved these changes
EdCaunt EdCaunt approved these changes

Assignees

No one assigned

Labels

documentation examples examples

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL