Skip to content

fix(robot): improve jacob0_dot docstring, typing overloads, and representation error handling - #737

Merged
petercorke merged 3 commits into
petercorke:mainfrom
paneendrakumar0:fix-jacob0-dot-issue-734
Oct 14, 2026
Merged

petercorke merged 3 commits into
petercorke:mainfrom
paneendrakumar0:fix-jacob0-dot-issue-734

Conversation

@paneendrakumar0

Copy link
Copy Markdown
Contributor

Fixes #734

Description

This PR improves the documentation, typing, and validation of
Robot.jacob0_dot.

Documentation

  • Correct the method name and typographical errors in the docstring.
  • Clarify the method summary, parameter types, and return shape.
  • Use the standard Sphinx seealso directive and correct reference
    formatting.
  • Add a runnable example using the Puma560 robot.
  • Document that the analytical-representation path computes the
    Hessian numerically and is approximate and slower than the
    geometric Jacobian derivative path.

Typing and validation

  • Restrict the q=None overload to representation=None.
  • Raise a clear ValueError when representation is specified
    without providing q.
  • Remove obsolete commented-out exploratory code.

Tests

  • Test the precomputed J0 path with q=None.
  • Test all four analytical representations: rpy/xyz, rpy/zyx,
    eul, and exp.
  • Test the expected error when q=None and representation is set.

Testing

Full repository test suite:

  • 767 passed
  • 82 skipped
  • 17 subtests passed
  • 0 failures and 0 errors

Focused tests:

  • tests/test_jacob.py: 14 passed
  • tests/test_DHRobot.py: 79 passed
  • tests/test_ETS.py: 54 passed

Formatting and whitespace checks passed:

  • ruff format --check tests/test_jacob.py
  • git diff --check

…sentation error handling

Fixes petercorke#734

- Correct method name reference from robot.jacob_dot to robot.jacob0_dot
- Fix 'to to' typo in docstring
- Clarify time derivative of manipulator Jacobian in frame {0}
- Document q=None behavior and add type/shape annotations for q, qd, J0, and rtype
- Convert See Also section to Sphinx .. seealso:: directive
- Align continuation indent in References rubric to avoid blockquote rendering
- Add runnable pycon example in docstring
- Note that representation uses numerical Hessian via spatialmath numhess
- Update typing overloads to restrict representation=None when q=None
- Raise clear ValueError when representation is passed with q=None
- Remove commented-out dead exploratory code
- Add tests for precomputed J0 with q=None, all representations, and ValueError check
@codacy-production

codacy-production Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 13 complexity · 2 duplication

Metric Results
Complexity 13
Duplication 2

View in Codacy

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

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
src/roboticstoolbox/robot/Robot.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #737   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        143     143           
  Lines      14592   14594    +2     
=====================================
- Misses     14592   14594    +2     

☔ 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.

q: None,
qd: ArrayLike,
J0: NDArray = ...,
representation: L["rpy/xyz", "rpy/zyx", "eul", "exp"] | None = None,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this literal type removed?

Comment thread src/roboticstoolbox/robot/Robot.py Outdated
Time derivative of manipulator Jacobian in frame {0}

:param q: The joint configuration of the robot
:param q: The joint configuration of the robot (must be provided if

@petercorke petercorke Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably write this in terms of parameters being provided or omitted rather than being None.

param q: The joint configuration of the robot. Must be provided if
representation is specified; may be omitted if J0 is
provided and representation is not.
:type q: ArrayLike(n), optional

It might even be worth having a table that explains the relationship between representation, q and J0.

>>> robot = rtb.models.DH.Puma560()
>>> q = robot.qn
>>> qd = [0.1, -0.2, 0.3, -0.1, 0.2, -0.3]
>>> j0_dot = robot.jacob0_dot(q, qd)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for adding the runblock example, maybe do one for the J0 pass in as well.

@petercorke

Copy link
Copy Markdown
Owner

Thanks for your interest in RTB and helping to fix this method and adding more documentation and testing. I have some comments on your PR.

I'm not sure this completely closes out #734, and the rework I envisaged would aim for tougher testing. The new tests, based on the old one, essentially just compare the results to a handwritten version of what's in the method. It would be more convincing to compare to a handworked (or sympy derived) Jacobian and Hessian for say a simple RR robot.

@paneendrakumar0

Copy link
Copy Markdown
Contributor Author

Hi @petercorke, thank you very much for the thoughtful and detailed review!

I have updated the PR with commit 6add4c9 addressing all of your feedback:

  1. RR Planar Ground-Truth Testing:

    • Added test_jacob0_dot_planar2_handworked_and_sympy in tests/test_jacob.py.
    • Compares robot.jacob0(q), robot.hessian0(q), robot.jacob0_dot(q, qd), and robot.jacob0_dot(None, qd, J0=J0) against both handworked closed-form equations and independently derived SymPy symbolic equations for rtb.models.DH.Planar2() across multiple configurations and joint velocities.
  2. Parameter Documentation & Table:

    • Rephrased :param q: and :type q: in terms of provided/omitted arguments as suggested.
    • Added a .. list-table:: in the docstring clearly detailing the interaction between representation, q, and J0.
  3. Precomputed J0 Runblock Example:

    • Added an example in the docstring's .. runblock:: pycon demonstrating computing J0 = robot.jacob0(q) and passing it into robot.jacob0_dot(None, qd, J0=J0).
  4. Typing Overload:

    • Restored representation: L["rpy/xyz", "rpy/zyx", "eul", "exp"] | None = None on the overload.

All 767 existing tests continue to pass alongside the new ground-truth tests. Ready for your review!

@petercorke
petercorke merged commit b83b469 into petercorke:main Oct 14, 2026
24 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 14, 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.

jacob0_dot: docstring errors, and analytic-representation path is a numerical Hessian

2 participants