Repository navigation
fix(lite): the notebooks install the wheel built with the site, and a browser check proves it - #863
Merged
Merged
Conversation
… browser check proves it The bootstrap looked for the bundled wheel in /pypi, which does not exist in the kernel, so JupyterLite always installed the last release from PyPI. Install through piplite, which looks in the site's own index first. The Node check stands in for piplite and also plots a trajectory under ipympl; a new Playwright check builds a site with the wheel and checks the install, ipywidgets, an ipympl figure and an animation in Chromium. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 minor 6 high 4 critical 2 medium |
🟢 Metrics 39 complexity · 0 duplication
Metric Results Complexity 39 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #863 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 146 146
Lines 17164 17164
=====================================
Misses 17164 17164 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ik_benchmark.ipynb was deleted on main, so its regenerated bootstrap cell goes too, and docs.yml no longer needs to exclude it from the JupyterLite content. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The JupyterLite site has been installing the last release of the toolbox from PyPI, not the wheel built with it, since #848. This fixes that, and adds the checks that would have caught it. No change to the toolbox.
The bug
The bootstrap cell (#848, copied from MVTB's) preferred "the wheel bundled with this site" using
Path("/pypi").glob(...). There is no/pypifolder in a JupyterLite kernel (checked in Chromium with the pinned kernel 0.6.1), so that branch never ran and it always didmicropip.install("roboticstoolbox-python")from PyPI. "Try it Now" therefore did not trackmain, and fixes such as #860 were not in the site. The oldpiplite.install(...)cells did pick up the bundled wheel, becausepiplitelooks in the site's own index first, so #848 regressed this. MVTB has the same code: petercorke/machinevision-toolbox-python#160.Nothing noticed, because the Node check mounted the wheel in Pyodide's filesystem and redirected the
pypi/install to it.The fix
In the browser, install through
piplite(the site's index first), falling back tomicropipwhen there is nopiplite, for example under Node. The generated bootstrap cell is regenerated in all 13 notebooks; nothing else in them changes.The checks
docs/lite/wasm_check/): a stand-inpipliteinstalls the wheel (replacing thepypi/redirect), and a new step plots a trajectory under theipymplbackend in real Pyodide and checks that the inline mode is chosen and a different image is pushed for every step. It fails with the code from before fix(plot): a trajectory is animated whatever %matplotlib says #860 (AssertionError: notebook-widget).docs/lite/browser_check/, new, Playwright and Chromium): builds a small site with the wheel and one notebook, runs it, and requires the bootstrap line; the hash of the installedPyPlot.pyto equal the checkout's (so the wheel built here is the one installed); anipywidgetsslider; anipymplfigure of one pose; and at least 5 different images while an animation cell runs. It is adocs.ymlstep after the site is built, informational (continue-on-error) until it has passed once onmain, with the screenshot kept as an artifact on failure. Run locally it passes in about 2 minutes, and fails with the old bootstrap (the toolbox in the browser is not the wheel built from this checkout ... installed the last release from PyPI).This also confirms in a real JupyterLite site that
ipywidgetsandipymplwork there, and that a trajectory animates with the fix.Please note
Merging this changes the generated bootstrap cell, and only that cell, in every notebook. It merges cleanly into a branch that has not edited that cell.
18 tests in
tests/test_notebook_bootstrap.py(one new: the bootstrap installs throughpipliteand not/pypi). I have not seen the new CI step run in GitHub Actions yet. Written with AI assistance (Claude Code).🤖 Generated with Claude Code