Skip to content

Fix frame labels in chapter 2 notebook - #50

Merged
petercorke merged 1 commit into
petercorke:mainfrom
0xrohitgarg:codex/fix-chap2-frame-labels
Sep 26, 2026
Merged

petercorke merged 1 commit into
petercorke:mainfrom
0xrohitgarg:codex/fix-chap2-frame-labels

Conversation

@0xrohitgarg

Copy link
Copy Markdown
Contributor

Replace framelabel with frame in the three trplot2 calls in Chapter 2's rotation example so the RX, XR and XC labels render correctly.

Verified the edited cell runs and renders all three labels with SpatialMath 1.1.18. Full notebook suite not run.

@petercorke

Copy link
Copy Markdown
Owner

Thanks for this. It led to PRs for trplot2() and trplot() which didn't pick up the incorrect option name. Merging this now.

@petercorke
petercorke merged commit f2bf2a3 into petercorke:main Sep 26, 2026
taughz pushed a commit to rai-opensource/spatialmath-python that referenced this pull request Oct 16, 2026
## Summary

`trplot()` and `trplot2()` both ended with a bare `**kwargs` that was
never read anywhere in their bodies, so a typo'd keyword (e.g.
`framelabel=` instead of `frame=`) was silently accepted and ignored
instead of raising — it drew the frame with no error and no label at
all. This surfaced downstream in
[petercorke/RVC3-python#50](petercorke/RVC3-python#50),
where a notebook had exactly this typo for years with no test ever
catching it.

- Removed the dead `**kwargs` sink from both functions so an
unrecognized keyword now raises `TypeError`, same as any normal Python
call.
- `trplot`'s `anaglyph` branch only forwarded a small fixed subset of
parameters to each eye's recursive call, silently dropping `textcolor`,
`labels`, `originsize`, `origincolor`, `axislabel`, `axissubscript`,
`width` and `projection`. It also never returned `ax` and never actually
called `plt.show(block=...)`, so anaglyph mode ignored `block` entirely.
Fixed to forward the full applicable parameter set, return `ax`, and
call `plt.show()` once after both eyes are drawn.
- `trplot`'s "T is an iterable of transforms" branch forwarded every
parameter except `axissubscript`, and also never returned `ax`. Fixed.
- `tranimate`/`tranimate2` were splatting the *same* unfiltered kwargs
dict into `Animate()`/`Animate2()`'s constructor, the drawing call, and
`run()`'s animation-control call, relying on each one's own dead
`**kwargs` sink to drop what it didn't need. Now that `trplot`/`trplot2`
validate strictly, `run()`-only parameters (`movie`, `repeat`,
`interval`, `nframes`, `wait`) are split off before the drawing call.
- Fixed a long-standing typo in `test_pose2d.py::test_graphics`
(`T0=T2`, which was never a real parameter — `animate()`'s documented
one is `start=`) that this same silent-swallow bug had been quietly
masking since the test was written.

## New test coverage

Added `tests/base/test_animate.py`. While chasing the
`tranimate`/`tranimate2` regression above, I found that constructing a
`FuncAnimation` under a non-interactive backend (the Agg backend this
repo's whole test suite runs under) doesn't actually run its per-frame
`update()` callback — confirmed directly by checking `Animate2`'s
internal frame counter, which never advances past its initial value
unless something drives the animation. So the existing
`animate()`/`tranimate()` tests only ever proved construction didn't
raise, not that interpolation/drawing works.

The new tests force every frame through the real callback via
`FuncAnimation.save()` with `PillowWriter` (pure Python, no external
`ffmpeg` dependency) and assert `trinterp`/`trinterp2` actually ran
across a real `s: 0→1` sweep. This is what caught the
`tranimate`/`tranimate2` kwargs-splitting bug above in the first place.

## Test plan

- [x] Full test suite passes: `353 passed, 3 skipped` (skips are
pre-existing, display-dependent)
- [x] Fail/fix/pass verified for every behavioral change (confirmed each
new/modified test fails against the pre-fix code for the intended
reason, and passes after)
- [x] `SE3`/`SE2` `.plot()`/`.animate()` wrapper methods (which forward
`*args, **kwargs` straight through) verified to propagate the stricter
validation correctly

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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