Skip to content

[roottest] Use nbconvert as a library in nbdiff.py#20456

Merged
guitargeek merged 4 commits intoroot-project:masterfrom
guitargeek:nbdiff
Nov 19, 2025
Merged

[roottest] Use nbconvert as a library in nbdiff.py#20456
guitargeek merged 4 commits intoroot-project:masterfrom
guitargeek:nbdiff

Conversation

@guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Nov 18, 2025

Using nbconvert as a Python library is less fragile, because otherwise there is ambiguity how the command line util should be called (the name changed over the versions as it migrated from ipython to jupyter).

This will make it possible to run the notebook tests also on the Fedora Rawhide build that is linked against the Python debug version, and hopefully also fix the test failures on the macOS platforms when a Python interpreter other than the system Python is used.

@guitargeek guitargeek requested a review from dpiparo as a code owner November 18, 2025 15:10
@guitargeek guitargeek self-assigned this Nov 18, 2025
@github-actions
Copy link

github-actions bot commented Nov 18, 2025

Test Results

    21 files      21 suites   3d 18h 30m 47s ⏱️
 3 776 tests  3 431 ✅   0 💤 345 ❌
77 432 runs  76 699 ✅ 388 💤 345 ❌

For more details on these failures, see this check.

Results for commit 8b6fb40.

♻️ This comment has been updated with latest results.

@guitargeek guitargeek force-pushed the nbdiff branch 3 times, most recently from b2c687b to 08a8640 Compare November 19, 2025 05:04
@guitargeek guitargeek changed the title [roottest] Inherit python executable from main process in nbdiff.py [roottest] Use nbconvert as a library in nbdiff.py Nov 19, 2025
Using nbconvert as a Python library is less fragile, because otherwise
there is ambiguity how the command line util should be called (the name
changed over the versions as it migrated from ipython to jupyter).
Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

LFTM (but see the set of questions)

We only support Python 3 at this point. By updating the reference
notebooks to have the Python 3 kernel in the metainfo, we don't need to
do extra steps in the `nbdiff.py` script, mapping injecting the
`python3` kernel name manually.
@guitargeek
Copy link
Contributor Author

Thanks for the review! I have added one commit that addresses your comments. In particular, the reference notebooks are not updated to Python 3, so we don't have to manually set the kernel name in the nbdiff.py script.

@guitargeek
Copy link
Contributor Author

The failures in Fedora Rawhide are unrelated: they come from an update of the system gtest, and because the builds are incremental it still tries to link against the old version.

Maybe it's possible to also "hash" the docker image somehow, and when it changed do a clean build. To be seen later if this should continue to annoy us.

@guitargeek guitargeek merged commit 17182ef into root-project:master Nov 19, 2025
27 of 30 checks passed
@guitargeek guitargeek deleted the nbdiff branch November 19, 2025 13:44
guitargeek added a commit to root-project/roottest that referenced this pull request Nov 19, 2025
guitargeek added a commit to guitargeek/root-ci-images that referenced this pull request Nov 20, 2025
This reverts commit 2e9c9e6.

Things might work, now that we're using nbconvert as a library:
root-project/root#20456
guitargeek added a commit to root-project/root-ci-images that referenced this pull request Nov 20, 2025
This reverts commit 2e9c9e6.

Things might work, now that we're using nbconvert as a library:
root-project/root#20456
guitargeek added a commit to root-project/roottest that referenced this pull request Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants