fix roboticstoolbox optionality claim for visualize_trajectory.py - #1
Open
Kajko25 wants to merge 2 commits into
Open
fix roboticstoolbox optionality claim for visualize_trajectory.py#1Kajko25 wants to merge 2 commits into
Kajko25 wants to merge 2 commits into
Conversation
|
Confirmed the claim in the PR body against the code: |
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 README currently promises:
That holds for
smooth_resampled_traj.py, which imports it defensively and gates the FK path on_RTB_AVAILABLE:visualize_trajectory.pyimports it unguarded at module level (line 11), so without the package it raisesModuleNotFoundErrorbefore any of its code runs — it does not fall back, it fails to load.I deliberately did not add a
try/exceptthere. That script is 138 lines with a single plotting function that consumes forward-kinematics positions; end-effector space is the whole point of it, so there is no joint-space path to fall back to. Faking one would mean writing a feature rather than fixing a defect. The accurate fix is to scope the README claim to the tools that actually implement the fallback, and to say plainly on the tool's own section that it requires the package.Also removed a dead exception handler in
compute_fk:The handler re-runs the exact call that just failed, so it cannot recover anything; the bare
exceptadditionally swallowsKeyboardInterruptandSystemExit. Replaced with the plain call.Disclosure on how this was verified: I read the code rather than executing it — I had no environment to install the dependencies into, so there is no traceback attached. The conclusion rests on Python's import semantics for an unguarded top-level
importand on the contrast with the sibling file's guarded import. If you would rathervisualize_trajectory.pydegrade gracefully instead of documenting the requirement, that is a different and larger change and I am happy to look at it separately.Two files, +4/−4.
visualize_trajectory.pystill compiles.