Skip to content

feat(pose,quaternion): add __imatmul__ (@=) operator - #199

Open
petercorke wants to merge 2 commits into
rai-opensource:masterfrom
petercorke:feat/imatmul
Open

feat(pose,quaternion): add __imatmul__ (@=) operator#199
petercorke wants to merge 2 commits into
rai-opensource:masterfrom
petercorke:feat/imatmul

Conversation

@petercorke

Copy link
Copy Markdown
Collaborator

What

X @= Y now works as the augmented-assignment form of the existing X @ Y
(__matmul__, which composes with normalization) — previously only X *= Y
existed, which composes without normalization. Useful when a pose or unit
quaternion is updated incrementally over many cycles and you want the
normalized form without writing X = X @ Y by hand.

Added to BasePoseMatrix (covers SO2/SE2/SO3/SE3) and Quaternion
(covers Quaternion/UnitQuaternion).

Note on semantics: like the existing __imul__, this doesn't mutate the
object in place — __imatmul__ returns a new (normalized) object and Python
rebinds the name, same as X = X @ Y. That matches the existing *=
pattern in this codebase exactly, just saves writing X = X @ Y.

A bug found while adding test coverage

This started as a cherry-pick of old, never-merged WIP work. While writing
tests I found Quaternion.__imatmul__'s docstring claimed q1 @= q2 sets
q1 := qnorm(q1 * q2), but the implementation just delegated to __mul__
— identical to plain *=, no normalization at all, contradicting both the
docstring and the entire point of adding @=.

UnitQuaternion.__matmul__ (pre-existing, unchanged) already normalizes
correctly via smb.qunit(smb.qqmul(x, y))qunit being the actual
normalizer; qnorm just returns the scalar magnitude, so it was never
really the right function despite the docstring's wording.

Fixed by having __imatmul__ delegate to left @ right instead of
left.__mul__(right). Deliberately not left.__matmul__(right) either:
plain Quaternion has no __matmul__ (only UnitQuaternion defines one),
and calling the dunder directly as a plain attribute bypasses Python's
normal operator fallback, raising a confusing AttributeError instead of
the TypeError that q1 @ q2 already raises consistently for plain
Quaternion. left @ right matches @'s behaviour exactly in both cases.

Also fixed the docstring's -> bool return type annotation (should be
-> Quaternion) and its example, which called Quaternion.Eul() — a
method that only exists on UnitQuaternion.

Testing

  • Added @= coverage for SO3/SE3 (must match @) alongside the
    existing *= tests in test_pose3d.py.
  • Added @= coverage for UnitQuaternion (must match @, not *) and for
    plain Quaternion (must raise TypeError, matching @, not silently
    degrade to *=) in test_quaternion.py.
  • Full suite: 338 passed, 4 skipped.
  • black --check clean at the pinned 23.10.0.

`X @= Y` now compounds poses/quaternions in place with automatic
normalization, mirroring the existing `X @ Y` (__matmul__) behaviour
but writing the normalized result back into the left operand instead
of returning a new one. Useful when a pose is updated incrementally
over many cycles and you don't want to pay for a fresh object each
time.

Added to BasePoseMatrix (covers SO2/SE2/SO3/SE3) and Quaternion
(covers Quaternion/UnitQuaternion).
Quaternion.__imatmul__'s own docstring claimed `q1 @= q2` sets
`q1 := qnorm(q1 * q2)`, but the implementation just delegated to
__mul__ - identical to plain *=, no normalization at all, contradicting
both the docstring and the entire point of adding a separate @=
operator. UnitQuaternion.__matmul__ (pre-existing, unchanged) already
does this correctly via smb.qunit(smb.qqmul(x, y)) - qunit being the
normalizer; qnorm just returns the scalar magnitude, so it was never
actually the right function despite the docstring's wording.

Fixed by having __imatmul__ delegate to `left @ right` instead of
left.__mul__(right). Deliberately not left.__matmul__(right): plain
Quaternion has no __matmul__ (only UnitQuaternion defines one, with
normalization), and calling the dunder directly as a plain attribute
bypasses Python's normal operator fallback, raising a confusing
AttributeError instead of the same TypeError `q1 @ q2` already raises
for plain Quaternion. `left @ right` matches @'s behaviour exactly in
both cases: normalizes for UnitQuaternion, raises consistently for
Quaternion. Also fixed the docstring's `-> bool` return type (should
be `-> Quaternion`) and its example, which used Quaternion.Eul() - a
method that only exists on UnitQuaternion.

Tests: added @= coverage for UnitQuaternion (must match @, not *) and
for plain Quaternion (must raise TypeError, matching @, not silently
degrade to *=).
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

2 participants