Skip to content

fix(ik): unify ik_XX/ikine_XX return types and fix IK docs - #652

Merged
petercorke merged 7 commits into
mainfrom
fix/ik-return-type-docs
Aug 26, 2026
Merged

fix(ik): unify ik_XX/ikine_XX return types and fix IK docs#652
petercorke merged 7 commits into
mainfrom
fix/ik-return-type-docs

Conversation

@petercorke

Copy link
Copy Markdown
Owner

Summary

  • ik_LM/ik_NR/ik_GN (fast, C++-backed) returned a bare 5-tuple while ikine_LM/ikine_NR/ikine_GN/ikine_QP (pure-Python) already returned IKSolution -- same family of methods, two different return shapes. Wrapped the C++ tuple in IKSolution (in ETS.py and RobotKinematics.py's forwarders), and updated return-type annotations and :returns:/:rtype: docs to match.
  • Added IKSolution.__getitem__ (matching __iter__'s order) and a __repr__ matching the existing __str__ -- previously sol[0] raised TypeError, and the default dataclass repr was inconsistent with __str__.
  • Added a loud .. warning:: to ik_LM/ik_NR/ik_GN's docstrings: they require the compiled C++ extension and raise RuntimeError without it (pure-Python builds, Pyodide/JupyterLite) -- use ikine_LM/ikine_NR/ikine_GN instead there.
  • Made every ik_XX/ikine_XX pair bidirectionally cross-reference each other (previously ik_XX only referenced its C++ siblings, and vice versa).
  • Fixed "Levemberg-Marquadt"/"Marquadt" -> "Levenberg-Marquardt" (widespread misspelling across IK.py, RobotKinematics.py, DHRobot.py, and two docs files) plus a few other typos ("deined" -> "defined", "progamming" -> "programming").
  • Found and fixed several copy-paste bugs while doing this review: RobotKinematics.ik_GN's own "See Also" listed itself instead of ik_LM/ik_NR; ikine_GN's and ikine_QP's "See Also" referenced the wrong solver class entirely (IK_NR instead of IK_GN/IK_QP); two runblock examples said "ikine_GN"/"ikine_LM" in prose while actually calling ik_NR/ik_LM/ik_GN.
  • Found and removed 5 completely broken, dead methods: DHRobot.ik_lm_chan/ik_lm_wampler/ik_lm_sugihara/ik_nr/ik_gn all forwarded to self.ets().<same-name>(...), but ETS has never had methods by these names -- every one of them raises AttributeError unconditionally, confirmed by direct testing. Their docstrings even had literal :seealso: TODO placeholders. No test exercised any of them.
  • Fixed examples/ik_exp.py, the only caller of those dead ETS-level method names (ets.ik_nr/ets.ik_gn/ets.ik_lm_chan/etc.), to use the real ik_NR/ik_GN/ik_LM(method=...) API -- also fixed a 5-tuple unpacking that would have silently broken against the new 6-field IKSolution, and dropped several unused imports (fknm, swift, spatialgeometry, sys).

Item 3 of a 4-item IK-solver cleanup plan (see claude-notes/ik-solver-cpp-python-divergence.md). Item 1 (YuMi gripper parent-swap) is PR #649, Item 2 (broken example references) is PR #650.

Test plan

  • New regression tests: IKSolution.__getitem__/__repr__, and ik_LM/ik_NR/ik_GN return real IKSolution instances (isinstance checks)
  • Full test suite green (731 passed, 18 skipped) in a clean isolated venv, confirmed both before and after the dead-method removal
  • Confirmed (by actually reverting each fix) that the 5 dead DHRobot methods really do raise AttributeError unconditionally, and that ik_exp.py runs end-to-end after the rewrite
  • Sphinx docs build (-W --keep-going) clean of any new warnings -- only the same 2 pre-existing, unrelated roboticstoolbox.tools.trchain import warnings remain

🤖 Generated with Claude Code

petercorke and others added 7 commits August 26, 2026 21:09
IKSolution had __iter__ but no __getitem__, so positional indexing
(sol[0], sol[1], ...) -- the pattern every existing caller and the
old bare-tuple return used -- raised TypeError. Add __getitem__
matching __iter__'s order, and a __repr__ matching the existing
custom __str__ instead of the verbose default dataclass repr.

Also fixes "Levemberg-Marquadt"/"Marquadt" -> "Levenberg-Marquardt"
and "progamming" -> "programming", present throughout this file's
docstrings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ik_LM/ik_NR/ik_GN (the fast C++-backed solvers) returned a bare
5-tuple while ikine_LM/ikine_NR/ikine_GN (the pure-Python solvers)
already returned IKSolution -- same family of methods, two different
return shapes. Wrap the C++ tuple in IKSolution in both ETS.ik_LM/
ik_NR/ik_GN and their RobotKinematics forwarders, and update the
return-type annotations and :returns:/:rtype: docstring fields to
match (ikine_LM/ikine_NR/ikine_GN/ikine_QP were missing :returns:/
:rtype: entirely -- added those too).

Also:
- bidirectionally cross-reference each ik_XX with its ikine_XX
  counterpart (previously only cross-referenced their C++ siblings)
- add a loud warning to ik_LM/ik_NR/ik_GN's docstrings that they
  require the compiled C++ extension and raise RuntimeError without
  it (e.g. pure-Python builds, Pyodide/JupyterLite)
- fix several copy-paste bugs in RobotKinematics.py found while doing
  this: ik_GN's own "See Also" listed itself instead of ik_LM/ik_NR,
  ikine_GN's and ikine_QP's listed the wrong solver class entirely
  (IK_NR instead of IK_GN/IK_QP), and two runblock examples said
  "ikine_GN"/"ikine_LM" while actually calling ik_NR/ik_LM/ik_GN
- fix "Levemberg-Marquadt"/"Marquadt" -> "Levenberg-Marquardt" and
  "deined" -> "defined" throughout both files

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DHRobot.ik_lm_chan/ik_lm_wampler/ik_lm_sugihara/ik_nr/ik_gn all
forwarded to self.ets().<same-name>(...), but ETS has never had
methods by these names (only the unified ik_LM/ik_NR/ik_GN, each
taking a method= kwarg where relevant) -- every one of these five
methods raises AttributeError unconditionally on any call. Their
docstrings even have literal ":seealso: TODO" placeholders. No test
exercises any of them.

Confirmed dead: nothing in tests/ or docs/ references any of the
five; the only caller was examples/ik_exp.py (fixed separately).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Called the now-removed dead ets.ik_nr/ik_gn/ik_lm_chan/ik_lm_wampler/
ik_lm_sugihara methods, which never existed on ETS in the first place
(same root cause as the DHRobot dead-method removal). Rewired to the
real ik_NR/ik_GN/ik_LM(method=...) API with matching parameter names,
switched from raw 5-tuple unpacking to IKSolution attribute access
(the old unpacking would have silently broken now that these methods
return a 6-field IKSolution instead of a 5-tuple), and dropped several
entirely unused imports (fknm, swift, spatialgeometry, sys, and unused
typing names).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers IKSolution.__getitem__/__repr__, and that ik_LM/ik_NR/ik_GN
now return real IKSolution instances rather than a bare tuple.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… verify #379 repro

# Conflicts:
#	tests/test_IK.py
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (6691113) to head (6d85262).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/roboticstoolbox/ets/ETS.py 0.00% 7 Missing ⚠️
src/roboticstoolbox/robot/IK.py 0.00% 4 Missing ⚠️
src/roboticstoolbox/robot/RobotKinematics.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #652   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        143     143           
  Lines      14026   14027    +1     
=====================================
- Misses     14026   14027    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@petercorke
petercorke merged commit 07a60f4 into main Aug 26, 2026
23 checks passed
@petercorke
petercorke deleted the fix/ik-return-type-docs branch August 26, 2026 11:40
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
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.

1 participant