From d61729c38db2174fbe6933f59a9d85447a03339e Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Tue, 25 Aug 2026 11:29:55 +1000 Subject: [PATCH 1/3] build: add RTB_PURE_WHEEL to build a genuine py3-none-any wheel CMakeLists.txt: RTB_BUILD_EXTENSIONS option (default ON) wraps both nanobind_add_module calls -- set -DRTB_BUILD_EXTENSIONS=OFF to skip them. Both _fknm_c and _frne_c already have complete, tested pure-Python fallbacks (fknm.py/frne.py, tests/test_fknm_fallback.py), so this is a real working configuration. pyproject.toml: RTB_PURE_WHEEL=1 env var triggers a scikit-build-core override (wheel.platlib=false) that forces the wheel tag to py3-none-any, confirmed by reading scikit-build-core's own wheel_tag.py. Combined with the CMake option above (via cmake.define), this replaces cross-compiling a wasm32 binary via cibuildwheel's Pyodide platform -- which coupled the wheel to one specific Pyodide/Emscripten ABI version and to Pyodide's JSPI-default-on cutover (Safari/Firefox lack JSPI) -- with an ordinary wheel built on a normal runner. Verified locally: RTB_PURE_WHEEL=1 build produces roboticstoolbox_python-1.4.0-py3-none-any.whl with zero compiled targets ("ninja: no work to do"), installs and runs correctly (fkine, jacob0, ikine_LM all verified against a fresh venv), and a normal build without the env var is unaffected (still produces a compiled cp312 wheel). --- CMakeLists.txt | 69 ++++++++++++++++++++++++++++++-------------------- pyproject.toml | 49 +++++++++++++++++------------------ 2 files changed, 65 insertions(+), 53 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ee719ddf7..d0694675f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,32 +15,47 @@ execute_process( ) find_package(nanobind CONFIG REQUIRED HINTS "${NB_DIR}") -# --------------------------------------------------------------------------- -# _frne_c — Newton-Euler inverse dynamics -# ne.c / vmath.c are pure C maths; frne_nb.cpp is the nanobind glue. -# --------------------------------------------------------------------------- -nanobind_add_module(_frne_c - src/roboticstoolbox/robot/cpp-extensions/vmath.c - src/roboticstoolbox/robot/cpp-extensions/ne.c - src/roboticstoolbox/robot/cpp-extensions/frne_nb.cpp -) -target_include_directories(_frne_c PRIVATE - src/roboticstoolbox/robot/cpp-extensions -) +# Set -DRTB_BUILD_EXTENSIONS=OFF to skip both compiled extensions and build a +# genuinely pure-Python wheel instead -- both _fknm_c and _frne_c have +# complete, tested pure-Python fallbacks (see fknm.py/frne.py and +# tests/test_fknm_fallback.py), so this is a real, working configuration, +# not just a stub. Used for the Pyodide/JupyterLite wheel (see release.yml's +# build_pyodide job and pyproject.toml's scikit-build-core override): rather +# than cross-compiling for wasm32 via cibuildwheel's Pyodide platform (which +# ties the wheel to a specific Pyodide/Emscripten ABI version -- see git +# history on this file/release.yml for the JSPI/tag saga that caused), +# building on a normal runner with extensions off produces an ordinary +# py3-none-any wheel with no such coupling at all. +option(RTB_BUILD_EXTENSIONS "Build the compiled _fknm_c/_frne_c nanobind extensions" ON) -# --------------------------------------------------------------------------- -# _fknm_c — forward kinematics, Jacobian, Hessian, IK -# Eigen is vendored as header-only in src/roboticstoolbox/ets/cpp-extensions/Eigen/ -# fknm_nb.cpp is the nanobind glue; maths lives in methods/ik/linalg. -# --------------------------------------------------------------------------- -nanobind_add_module(_fknm_c - src/roboticstoolbox/ets/cpp-extensions/methods.cpp - src/roboticstoolbox/ets/cpp-extensions/ik.cpp - src/roboticstoolbox/ets/cpp-extensions/linalg.cpp - src/roboticstoolbox/ets/cpp-extensions/fknm_nb.cpp -) -target_include_directories(_fknm_c PRIVATE - src/roboticstoolbox/ets/cpp-extensions -) +if(RTB_BUILD_EXTENSIONS) + # ----------------------------------------------------------------------- + # _frne_c — Newton-Euler inverse dynamics + # ne.c / vmath.c are pure C maths; frne_nb.cpp is the nanobind glue. + # ----------------------------------------------------------------------- + nanobind_add_module(_frne_c + src/roboticstoolbox/robot/cpp-extensions/vmath.c + src/roboticstoolbox/robot/cpp-extensions/ne.c + src/roboticstoolbox/robot/cpp-extensions/frne_nb.cpp + ) + target_include_directories(_frne_c PRIVATE + src/roboticstoolbox/robot/cpp-extensions + ) + + # ----------------------------------------------------------------------- + # _fknm_c — forward kinematics, Jacobian, Hessian, IK + # Eigen is vendored as header-only in src/roboticstoolbox/ets/cpp-extensions/Eigen/ + # fknm_nb.cpp is the nanobind glue; maths lives in methods/ik/linalg. + # ----------------------------------------------------------------------- + nanobind_add_module(_fknm_c + src/roboticstoolbox/ets/cpp-extensions/methods.cpp + src/roboticstoolbox/ets/cpp-extensions/ik.cpp + src/roboticstoolbox/ets/cpp-extensions/linalg.cpp + src/roboticstoolbox/ets/cpp-extensions/fknm_nb.cpp + ) + target_include_directories(_fknm_c PRIVATE + src/roboticstoolbox/ets/cpp-extensions + ) -install(TARGETS _frne_c _fknm_c DESTINATION roboticstoolbox) + install(TARGETS _frne_c _fknm_c DESTINATION roboticstoolbox) +endif() diff --git a/pyproject.toml b/pyproject.toml index 905dd5ecc..7bb5171a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -145,6 +145,23 @@ sdist.include = [ ] sdist.exclude = ["rtb-data/**"] +# Set RTB_PURE_WHEEL=1 to build a genuinely pure-Python wheel: skips both +# compiled extensions (CMakeLists.txt's RTB_BUILD_EXTENSIONS option) and +# forces the wheel tag to py3-none-any (wheel.platlib=false — confirmed by +# reading scikit-build-core's own wheel_tag.py: root_is_purelib forces +# plats=["any"], abi="none", pyvers=["py3"]). Used for the Pyodide/ +# JupyterLite build (see release.yml's build_pyodide job) -- both _fknm_c +# and _frne_c have complete, tested pure-Python fallbacks (fknm.py/frne.py, +# tests/test_fknm_fallback.py), so there's nothing missing at runtime. +# pip always prefers a platform-specific compiled wheel over py3-none-any +# when both are published (packaging.tags ranks specific tags higher) -- +# this wheel is only ever selected where no compiled wheel exists, i.e. +# Pyodide today. +[[tool.scikit-build.overrides]] +if.env.RTB_PURE_WHEEL = true +wheel.platlib = false +cmake.define.RTB_BUILD_EXTENSIONS = "OFF" + [tool.ruff] line-length = 88 @@ -174,32 +191,12 @@ archs = [ [tool.cibuildwheel.windows] archs = ["AMD64"] -[tool.cibuildwheel.pyodide] -# Pyodide (wasm32-emscripten) — used by JupyterLite -# cibuildwheel manages the Emscripten SDK; scikit-build-core picks up -# the toolchain file automatically from the PYODIDE environment. -# -# Deliberately NO explicit `pyodide-version` override here -- the -# `build_pyodide` job in release.yml is pinned to cibuildwheel 3.4.1 -# specifically (not the 4.1.1 used for native wheels) so it self-resolves -# its own default (cp312/Pyodide 0.27.7), matching spatialgeometry's own -# config exactly. An explicit `pyodide-version = "0.27.7"` override was -# tried and failed here: cibuildwheel 3.4.1's bundled pyodide-build -# (0.33.0) rejected it outright as "not compatible" even though it's -# that same tool's own documented default -- the override and the tool's -# internal default-resolution logic aren't equivalent, and only the -# latter actually works. Don't add one back without testing against a -# real build first. -# -# Why cp312/0.27.x specifically: Pyodide only ships cp313+ support from -# 0.28.0a1 onward, by which point WebAssembly JSPI ("stack switching") -# was already on by default (since 0.27.7) -- and Safari has no JSPI -# support, Firefox only behind a flag. cp312/0.27.x is the newest -# JSPI-safe line. jupyterlite-pyodide-kernel's pin in ci.yml must match -# (jupyterlite-pyodide-kernel==0.6.1, bundling Pyodide 0.27.6) -- see -# that file's "Fetch pyodide wheel for JupyterLite" step, and the -# toolbox-maintainer skill's JupyterLite version-pinning section for the -# full JSPI/Safari story before changing any of this. +# No [tool.cibuildwheel.pyodide] section -- the wasm/Pyodide wheel is no +# longer built via cibuildwheel's Pyodide platform (cross-compiling for a +# specific Emscripten/Pyodide ABI version was the whole source of the +# JSPI/tag-matching saga, see git history on this file and release.yml). +# It's built as an ordinary pure-Python wheel on a normal runner instead -- +# see the RTB_PURE_WHEEL override above and release.yml's build_pyodide job. [tool.pytest.ini_options] From 3d6f9d7d99574e49d84a5b78374071e78ddf0c7b Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Tue, 25 Aug 2026 11:30:05 +1000 Subject: [PATCH 2/3] ci: build the pyodide/JupyterLite wheel as a plain pure-Python build release.yml's build_pyodide job no longer uses cibuildwheel's Pyodide platform (no Emscripten SDK, no cross-compilation) -- just a plain `python -m build --wheel` with RTB_PURE_WHEEL=1 on a normal ubuntu-latest runner. Since the resulting py3-none-any wheel has no ABI to be rejected over, upload_pypi's "exclude pyodide wheel" step and the whole upload_pyodide_asset job (GitHub-Release-asset workaround) are gone -- it just uploads to PyPI like every other wheel. ci.yml's docs-build job builds this wheel fresh from the checked-out source on every push instead of fetching one from the latest GitHub Release -- feasible now because there's no compilation involved, so it's fast. This also fixes an existing staleness gap: "Try it Now" now tracks bleeding-edge main instead of lagging behind the last release. spatialgeometry's own wasm wheel is untouched here (still fetched via cross-repo `gh release download`, still cp312-pyodide-tagged) -- SG hasn't adopted the same fix yet (tracked: jhavl/spatialgeometry#46). Updated that step's comment to reflect the new asymmetry accurately. docs/source/intro.rst's "available as a GitHub release resource" line was describing the now-removed upload_pyodide_asset mechanism; updated. --- .github/workflows/ci.yml | 74 ++++++++++----------------- .github/workflows/release.yml | 96 ++++++++++++----------------------- docs/source/intro.rst | 2 +- 3 files changed, 60 insertions(+), 112 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bb6f5a97..447956d19 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -184,44 +184,24 @@ jobs: cd docs && make SPHINXOPTS="--keep-going" html touch build/html/.nojekyll - - name: Fetch pyodide wheel for JupyterLite - # A plain `pip wheel`/`build` here would produce a native - # macOS/Linux wheel with the compiled _fknm_c/_frne_c extensions -- - # useless in Pyodide (WASM). release.yml already cross-compiles a - # genuine wasm32 wheel via cibuildwheel (CIBW_PLATFORM=pyodide), - # forced to cp312 (see release.yml's build_pyodide job and - # pyproject.toml's [tool.cibuildwheel.pyodide] comment) and - # attaches it to each GitHub Release, since PyPI rejects this old, - # pre-standard `pyodide_*` platform tag. This means the "Try it - # Now" experience tracks the latest *release*, not bleeding-edge - # main -- deliberate, since a full Pyodide/Emscripten rebuild on - # every docs push would be slow. - # - # Deliberately staying on cp312/the old tag rather than the newer - # PEP-783 `pyemscripten_*` tag PyPI now accepts: Pyodide only - # supports cp313+ from 0.28.0a1 onward, by which point WebAssembly - # JSPI ("stack switching") was already on by default (since - # 0.27.7) -- and Safari has no JSPI support, Firefox only behind a - # flag. cp312/0.27.x is the newest JSPI-safe line. See the - # toolbox-maintainer skill's JupyterLite version-pinning section - # before "modernizing" this away. - # - # The `--pattern '*cp312*pyodide*'` filter also guards against a - # real upstream bug (https://github.com/petercorke/robotics-toolbox-python/issues/579): - # jupyterlite-pyodide-kernel's all.json generator keys entries by - # (name, version) and *overwrites* rather than accumulates, so if a - # release ever carried wasm wheels for more than one CPython - # version, fetching more than one would silently drop all but the - # alphabetically-last from the piplite index -- no build error, - # just a runtime "can't find a pure Python 3 wheel" in the browser. - # release.yml's CIBW_BUILD restriction means only one wasm wheel - # (cp312) is ever built now, so this shouldn't trigger in practice, - # but the filter costs nothing to keep as a defence-in-depth guard. - env: - GH_TOKEN: ${{ github.token }} + - name: Build pure-Python wheel for JupyterLite + # RTB_PURE_WHEEL=1 (see pyproject.toml's scikit-build-core override) + # skips _fknm_c/_frne_c and forces a py3-none-any tag -- both have + # complete, tested pure-Python fallbacks, so nothing is missing at + # runtime. This used to require fetching a wasm32 wheel + # cross-compiled by release.yml's build_pyodide job (via + # cibuildwheel's Pyodide platform) from the latest GitHub Release, + # since building that wheel here on every docs push would've been + # slow, and PyPI rejected its old-format tag outright. A pure wheel + # needs neither: it's fast enough to build fresh on every push (no + # compilation at all, see build_pyodide's comment in release.yml), + # so "Try it Now" now tracks bleeding-edge main instead of lagging + # behind the last release, and py3-none-any needs no cross-repo + # GitHub Release fetch at all. run: | mkdir -p docs/lite/pypi - gh release download --repo "$GITHUB_REPOSITORY" --pattern '*cp312*pyodide*' --dir docs/lite/pypi --clobber + pip install build + RTB_PURE_WHEEL=1 python3 -m build --wheel --outdir docs/lite/pypi - name: Fetch spatialgeometry pyodide wheel for JupyterLite # spatialgeometry is an unconditional runtime dependency of @@ -229,17 +209,17 @@ jobs: # copy internally at 1.4.0), so `piplite.install` needs a wasm # wheel for it too, staged in the same local index. Fetched from # spatialgeometry's own GitHub Release (jhavl/spatialgeometry) -- - # it doesn't publish this to PyPI at all (tech debt raised - # upstream: jhavl/spatialgeometry#46, though note that issue's - # "publish via the new PEP-783 tag" suggestion has the same - # JSPI/Safari problem as RTB's own wheel above if it means moving - # off cp312). Matching RTB's own wheel above, this fetches the - # cp312 build specifically. Low-risk despite the cross-repo fetch: - # spatialgeometry's own pyproject.toml notes its CMake build skips - # the compiled nanobind extension entirely under Emscripten, so - # this wheel is genuinely pure Python -- no Pyodide ABI-version - # coupling to track here, unlike the JSPI/cp312 discipline the - # RTB wheel above needs. + # it doesn't publish this to PyPI at all. SG's own CMake build + # already skips its compiled nanobind extension entirely under + # Emscripten (same idea as RTB_PURE_WHEEL above), so this wheel's + # *content* is genuinely pure Python -- but SG hasn't adopted the + # scikit-build-core override that makes the wheel *itself* + # py3-none-any yet, so it's still tagged cp312-cp312-pyodide_wasm32 + # and still needs this cross-repo GitHub Release fetch rather than + # a plain PyPI install. Tech debt raised upstream: + # jhavl/spatialgeometry#46 -- once SG applies the same fix RTB just + # did, this step (and the cp312 filter) can go away in favour of a + # plain `pip download`/PyPI dependency resolution. env: GH_TOKEN: ${{ github.token }} run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9e3b26bea..c982f4a07 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -60,49 +60,43 @@ jobs: path: ./wheelhouse/*.whl build_pyodide: - name: Build Pyodide (wasm32) wheel + name: Build pure-Python wheel (for Pyodide/JupyterLite) needs: verify_version runs-on: ubuntu-latest - continue-on-error: true steps: - uses: actions/checkout@v7 - - name: Diagnostics + - uses: actions/setup-python@v7 + with: + python-version: "3.12" + + # Not a cross-compile: RTB_PURE_WHEEL=1 (see pyproject.toml's + # scikit-build-core override) skips both _fknm_c/_frne_c (they have + # complete, tested pure-Python fallbacks -- fknm.py/frne.py, + # tests/test_fknm_fallback.py) and forces the wheel tag to + # py3-none-any. No Emscripten SDK, no cibuildwheel Pyodide platform, + # no Pyodide-version coupling at all -- a plain wheel built on a + # normal runner. This replaced an earlier approach that cross-compiled + # a real wasm32 binary via cibuildwheel's Pyodide platform, which tied + # the wheel to one specific Pyodide/Emscripten ABI version and, worse, + # to a JSPI-safe-vs-unsafe split (Safari/Firefox lack WebAssembly JSPI, + # which Pyodide >=0.27.7 needs) -- see git history on this job and on + # pyproject.toml for that saga. A pure-Python wheel has no ABI to be + # wrong about, so none of that applies here. + - name: Build pure-Python wheel run: | - python3 --version - pip3 --version - pip3 show cibuildwheel 2>/dev/null || echo "cibuildwheel not installed locally (normal)" - - - name: Build Pyodide wheel - # Pinned to cibuildwheel 3.4.1 deliberately -- NOT the same version - # as the native wheel builds below (4.1.1) -- because the tag - # *format* pyodide-build emits turns out to be a property of the - # pyodide-build tool version, not of which Pyodide version it - # targets. Confirmed directly: forcing cibuildwheel 4.1.1 to build - # cp312/Pyodide 0.27.7 (via CIBW_BUILD + CIBW_ENABLE overrides) - # still produced a wheel tagged `pyemscripten_2024_0_wasm32` (the - # new PEP-783 tag), which Pyodide 0.27.6's micropip -- like every - # pre-PEP-783 Pyodide release -- hard-rejects with "Wheel was built - # with Emscripten vpyemscripten.2024.0 but Pyodide was built with - # Emscripten v3.1.58". There is no `pyodide-version` override that - # fixes this: cibuildwheel 4.1.1 cannot produce an old-tag wheel at - # all. cibuildwheel 3.4.1 predates the tag-format change and - # defaults to cp312/Pyodide 0.27.7 unprompted (matches - # spatialgeometry's own pyodide build, same version, same reason). - # - # This is why cp312/Pyodide 0.27.x matters at all: Pyodide only - # ships cp313+ support from 0.28.0a1 onward, by which point - # WebAssembly JSPI ("stack switching") was already on by default - # (since 0.27.7) -- and Safari has no JSPI support, Firefox only - # behind a flag. See pyproject.toml's [tool.cibuildwheel.pyodide] - # comment and the toolbox-maintainer skill's JupyterLite - # version-pinning section for the full story before changing this. - uses: pypa/cibuildwheel@v3.4.1 - env: - CIBW_PLATFORM: pyodide + pip install build + RTB_PURE_WHEEL=1 python3 -m build --wheel --outdir wheelhouse + + - name: Verify wheel is genuinely pure Python + run: | + ls wheelhouse/ + case "$(ls wheelhouse/)" in + *-py3-none-any.whl) echo "OK: py3-none-any" ;; + *) echo "ERROR: expected a py3-none-any wheel"; exit 1 ;; + esac - uses: actions/upload-artifact@v7 - if: success() with: name: wheels-pyodide path: ./wheelhouse/*.whl @@ -161,35 +155,9 @@ jobs: name: sdist path: dist - # Exclude the Pyodide wheel -- PyPI rejects the old, pre-standard - # `pyodide_*` platform tag this cp312 build produces. Deliberately - # staying on this tag (not the newer PEP-783 `pyemscripten_*` one) to - # keep the wasm build JSPI-safe for Safari/Firefox -- see the - # `build_pyodide` job's comment and the toolbox-maintainer skill's - # JupyterLite version-pinning section. It's attached as a GitHub - # Release asset in the next job instead. - - name: Remove Pyodide wheels from PyPI upload set - run: rm -f dist/*pyodide*.whl - + # The pyodide wheel (py3-none-any) uploads to PyPI like any other + # wheel now -- no tag rejection, no GitHub-Release-asset workaround. + # See build_pyodide's comment for why. - uses: pypa/gh-action-pypi-publish@release/v1 with: skip-existing: true - - upload_pyodide_asset: - name: Attach Pyodide wheel to GitHub release - needs: build_pyodide - if: github.event_name == 'release' && needs.build_pyodide.result == 'success' - runs-on: ubuntu-latest - permissions: - contents: write - steps: - - uses: actions/download-artifact@v8 - with: - name: wheels-pyodide - path: pyodide-dist - - - name: Upload Pyodide wheel as release asset - uses: softprops/action-gh-release@v3 - with: - tag_name: ${{ github.ref_name }} - files: pyodide-dist/*.whl diff --git a/docs/source/intro.rst b/docs/source/intro.rst index 3e481d02f..1e09fb779 100644 --- a/docs/source/intro.rst +++ b/docs/source/intro.rst @@ -658,7 +658,7 @@ to import them if the user attempts to exploit a functionality that requires it. If a dependency is not installed, a warning provides instructions on how to install it using ``pip``. C/C++ extensions are provided for recursive Newton-Euler dynamics and optimized forward and inverse kinematics for ETS defined robots. These -wheels are built by the GitHub CI actions. A pyodide wheel is also built for use in the browser and is available as a GitHub release resource. +wheels are built by the GitHub CI actions. A pure-Python wheel (using tested pure-Python fallbacks for the C/C++ functionality) is also built for use in the browser via Pyodide/JupyterLite, and is published to PyPI alongside the compiled wheels. Spatial math layer From d4d2af3a81dc18731019dbd04c562c588d628e81 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Tue, 25 Aug 2026 12:14:54 +1000 Subject: [PATCH 3/3] fix: two real bugs found by running the suite against the pure-Python path Cherry-picked from the stale, never-merged feat/pyodide-pure-wheel-v2 branch (pushed 2026-07-23, 109 commits behind main as of this cherry-pick -- too stale to merge as-is, but these two findings are real and still live on main). That branch ran the full test suite inside a real Pyodide environment with zero compiled extensions present; both bugs reproduce identically with just the C extension absent, no real Pyodide required. tests/test_ETS.py::test_insert: genuine off-by-one -- a 7-joint ETS (jindex 0-6) tested with a 6-element q. The C extension has no bounds checking, so q[6] silently read past the array end (undefined behaviour, happened not to crash); the pure-Python fallback correctly raises IndexError. Verified fail (IndexError against the pure wheel) -> fix (q now has 7 elements) -> pass (both the pure-Python and compiled paths). URDFRobot.py's _load_rd_module: the sys.platform == "emscripten" guard only checked inside `except Exception`, not `except ImportError`. GitPython's subprocess-spawn failure under Pyodide surfaces as a plain ImportError, which the candidates loop's `except ImportError: continue` treated as "try the next name" -- after exhausting every candidate this fell through to a misleading "model renamed"/"not found" error instead of the correct, actionable one. Fixed by checking the platform up front, before the loop, since the outcome doesn't depend on which candidate name is tried. Verified fail (misleading rename error) -> fix -> pass (correct "browser sandbox" error), both by loading the pre/post-fix file content directly (bypassing an unrelated editable-install path collision) and via a proper pytest run against a real editable install of this worktree. New regression test: tests/test_URDFRobot.py, same patch.object(sys, "platform", "emscripten") pattern already used in tests/test_collision.py. Two other findings from that branch (test_ET.py's ungated C-only assertions, a WASM-numpy-specific float tolerance in test_trajectory.py) were not ported -- not live bugs, and the second can't be reproduced without a real Pyodide/WASM numpy build. See #633 for the fuller discussion, including why the branch's real-Pyodide CI job itself wasn't ported (three of its four findings were actually pure-Python-path coverage gaps, not WASM-specific -- catchable much more cheaply now that build_pyodide produces a real installable pure-Python wheel). --- src/roboticstoolbox/models/URDF/URDFRobot.py | 45 +++++++++++--------- tests/test_ETS.py | 11 ++++- tests/test_URDFRobot.py | 37 ++++++++++++++++ 3 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 tests/test_URDFRobot.py diff --git a/src/roboticstoolbox/models/URDF/URDFRobot.py b/src/roboticstoolbox/models/URDF/URDFRobot.py index 805915f00..c2994df31 100644 --- a/src/roboticstoolbox/models/URDF/URDFRobot.py +++ b/src/roboticstoolbox/models/URDF/URDFRobot.py @@ -90,6 +90,31 @@ def _load_rd_module(robot_name: str): "URDF model." ) + # robot_descriptions clones a git repository (via GitPython, which shells + # out to a real git binary) the first time a given model is imported. + # Pyodide/JupyterLite has no subprocess execution and no git binary, so + # this always fails there -- not a bug, an environment limitation. + # Checked up front, before the candidates loop below, rather than caught + # per-attempt: GitPython's failure in this sandbox surfaces as a plain + # ImportError (message: "emscripten does not support processes"), which + # the loop's `except ImportError` treats as "this candidate name doesn't + # exist, try the next one" -- so after exhausting every candidate it fell + # through to a misleading "model not found"/"renamed" error instead of + # this one. The outcome here is deterministic regardless of which + # candidate name is tried, so there is nothing to gain by attempting the + # loop at all on this platform. + if sys.platform == "emscripten": + raise ValueError( + f"Toolbox uses {_rd_link()} to provide URDF robot models, " + "which clones a git repository on first use. That isn't " + "possible in this browser (Pyodide/JupyterLite) sandbox -- " + f'this is an expected limitation loading "{robot_name}" ' + "here, not a bug. Try a DH- or ETS-based model instead " + "(e.g. rtb.models.DH.Panda()), or run this notebook in a " + "regular Python environment to use robot_descriptions-" + "backed models." + ) + candidates = [f"{robot_name}_description", f"{robot_name}_official_description"] last_error: ImportError | None = None for candidate in candidates: @@ -103,26 +128,6 @@ def _load_rd_module(robot_name: str): except ImportError as e: last_error = e continue - except Exception as e: - # robot_descriptions clones a git repository (via GitPython, - # which shells out to a real git binary) the first time a given - # model is imported. Pyodide/JupyterLite has no subprocess - # execution and no git binary, so this always fails there -- - # not a bug, an environment limitation. The exact exception type - # depends on how GitPython fails in that sandbox, so this is - # deliberately broad, but only ever intercepts on Pyodide. - if sys.platform == "emscripten": - raise ValueError( - f"Toolbox uses {_rd_link()} to provide URDF robot models, " - "which clones a git repository on first use. That isn't " - "possible in this browser (Pyodide/JupyterLite) sandbox -- " - f'this is an expected limitation loading "{robot_name}" ' - "here, not a bug. Try a DH- or ETS-based model instead " - "(e.g. rtb.models.DH.Panda()), or run this notebook in a " - "regular Python environment to use robot_descriptions-" - "backed models." - ) from e - raise renamed_to = _find_rd_rename(robot_name, candidates) if renamed_to is not None: diff --git a/tests/test_ETS.py b/tests/test_ETS.py index 9022f3201..c860a4bd0 100644 --- a/tests/test_ETS.py +++ b/tests/test_ETS.py @@ -547,7 +547,16 @@ def test_compile(self): self.assertTrue(len(r) > len(r2)) def test_insert(self): - q = [1.0, 2, 3, 4, 5, 6] + # 7 joints (l0..l6, jindex 0-6) need 7 values -- q previously had + # only 6, an off-by-one that silently "worked" only because the C + # extension has no bounds checking (q[6] read past the end of the + # array, undefined behaviour that happened not to crash). Confirmed + # by comparing r.eval() with 6 vs 7 elements: they give genuinely + # different results, proving index 6 really is read and used, not + # just harmlessly ignored. Found via the pure-Python fallback + # (_python_fkine), which correctly raises IndexError instead of + # silently reading garbage. + q = [1.0, 2, 3, 4, 5, 6, 7] deg = np.pi / 180 mm = 1e-3 tool_offset = (103) * mm diff --git a/tests/test_URDFRobot.py b/tests/test_URDFRobot.py new file mode 100644 index 000000000..3d46c7e00 --- /dev/null +++ b/tests/test_URDFRobot.py @@ -0,0 +1,37 @@ +""" +Regression tests for URDFRobot's robot_descriptions loading guards. +""" + +import sys +import unittest +from unittest.mock import patch + +from roboticstoolbox.models.URDF.URDFRobot import _load_rd_module + + +class TestURDFRobotEnvironmentGuards(unittest.TestCase): + def test_pyodide_raises_actionable_error(self): + # On real Pyodide, robot_descriptions' GitPython-backed clone fails + # with a plain ImportError ("emscripten does not support + # processes"), not some other exception type -- simulate that here. + # A prior version of this guard only checked sys.platform inside + # `except Exception`, not `except ImportError`, so the loop's + # `except ImportError: continue` swallowed it and fell through to a + # misleading "model not found"/"renamed" error instead of this one. + def fake_import_module(name): + raise ImportError(f"emscripten does not support processes: {name}") + + with patch.object(sys, "platform", "emscripten"), patch( + "roboticstoolbox.models.URDF.URDFRobot.importlib.import_module", + side_effect=fake_import_module, + ): + with self.assertRaises(ValueError) as cm: + _load_rd_module("panda") + + self.assertIn("browser", str(cm.exception)) + self.assertNotIn("is now named", str(cm.exception)) + self.assertNotIn("can not be found", str(cm.exception)) + + +if __name__ == "__main__": + unittest.main()