Repository navigation
fix: remove vendored spatialgeometry, restore real dependency - #583
Merged
Merged
Conversation
The 1b522e6 vendoring ("bundle spatialgeometry as pure-Python") was a temporary workaround for depending on an unreleased git branch. It's now stale relative to both the current PyPI release and SG's own in-progress main, and Link.py's SceneGroup(scene_children=...) call broke against SG main's rename to a positional initlist/**kwargs signature -- this only surfaced in dev environments with SG's sibling checkout editable-installed, since a real `pip install roboticstoolbox-python` never saw the divergence (it always got the frozen vendored snapshot). Removes src/spatialgeometry/ and restores spatialgeometry as a normal dependency, pinned to a specific commit on SG's unreleased main (1.4 draft) since that's the API this code now targets -- switch to a normal ">=1.4.0" constraint once spatialgeometry actually cuts that release. Also fixes tests/test_collision.py's mesh dict tests, which used nonexistent placeholder filenames ("robot.stl", "test.stl") -- SG main added a real os.path.isfile() check in Mesh.__init__ that these never exercised before.
This was referenced Aug 9, 2026
rtbtool.py deliberately wildcard-imports from spatialmath/ spatialgeometry/roboticstoolbox to populate the interactive shell's namespace. SG's unreleased main added a new Path shape class, exported via __all__, which shadowed the earlier `from pathlib import Path` for the rest of the module -- the script's own internal filesystem-path handling (examples_path(), main()'s script-loading) started resolving to spatialgeometry.Path instead, crashing with "could not convert string to float" the moment a real path string hit it. Switches to `import pathlib` + fully-qualified pathlib.Path(...) at the two internal call sites, leaving the wildcard imports (and the bare `Path` available to someone typing at the rtbtool prompt) alone. Fixes #582. A naming discussion on spatialgeometry's side (Path isn't released yet, so renaming it there is still on the table -- matplotlib hit the same collision and never exported a bare Path either) is deferred to a separate session.
r2 = DHRobot([l1, l2, l3, l0]) reused link instances that r0/r1 had already wired with the opposite parent/child relationship (l0 root, l1 its child) -- SG's new scene-graph cycle detection correctly rejects this as a genuine conflict (a link's scene_parent is single-valued, so the same instance can't simultaneously be "l0 is r0's root" and "l0 is r2's last child"). isspherical() is purely DH-parameter-based (DHRobot.py), so fresh link instances with the same parameter values are equivalent for this test -- r0/r1 can still share (same relative order, no conflict), only r2 needs its own set. Fixes #581.
Link-sharing across robots should be fine -- links are meant to be stateless (desiderata.md). It only breaks today because Link/BaseLink inherits SceneNode, bolting single-valued scene-graph state onto what should be a pure kinematic-model object. Corrects the previous commit's framing, which read as if reusing link instances across robots was itself the problem.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #583 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 142 142
Lines 13788 13788
=====================================
Misses 13788 13788 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Summary
The
1b522e65vendoring ("bundle spatialgeometry as pure-Python") was atemporary workaround for depending on an unreleased git branch. It's now
stale relative to both the current PyPI release and SG's own in-progress
main, andLink.py'sSceneGroup(scene_children=...)call broke againstSG
main's rename to a positionalinitlist/**kwargssignature -- thisonly surfaced in dev environments with SG's sibling checkout
editable-installed, since a real
pip install roboticstoolbox-pythonalwaysgot the frozen vendored snapshot instead.
Removes
src/spatialgeometry/and restoresspatialgeometryas a normalpyproject.tomldependency, pinned to a specific commit on SG's unreleasedmain(1.4 draft) since that's the API this code now targets -- switch to anormal
>=1.4.0constraint oncespatialgeometryactually cuts thatrelease.
Also fixes
tests/test_collision.py's mesh dict tests, which usednonexistent placeholder filenames (
"robot.stl","test.stl") -- SGmainadded a real
os.path.isfile()check inMesh.__init__that these neverexercised before.
Also fixes (found via CI after the initial push)
rtbtool.pywildcard-imports fromspatialgeometry, whoseunreleased
mainadded a newPathshape class that shadowed thescript's own
from pathlib import Path. Switched the internal usages tofully-qualified
pathlib.Path.test_issphericalreused linkinstances across differently-ordered
DHRobots, which SG's newscene-graph cycle detection now rejects. Worked around by giving the
reordered robot its own fresh link instances -- but this is a
workaround, not evidence the test was wrong: links are meant to be
stateless (
desiderata.md), so sharing instances across robots shouldbe fine. It only breaks because
Link/BaseLinkcurrently inheritsSceneNode, bolting single-valued scene-graph state onto what should bea pure kinematic-model object -- the real fix is Robot/Link architecture redesign: generic LinkType, one-Robot-class design, kinematic/scene-graph decoupling #571 (Robot/Link vs
scene-graph decoupling). DHRobot tests fail against SG's new scene-graph cycle detection when sharing link objects across robots #581 stays open; see the issue comment for the
full explanation.
Follow-up (filed separately, not blocking this PR)
ci.ymllikely still has a now-redundantpip install git+.../spatialgeometry.git@futurestep to check/removeTest plan
rtb.models.Panda()(and other URDF robots) construct again