diff --git a/spatialmath/base/transforms2d.py b/spatialmath/base/transforms2d.py index e9bc0e70..d09e820d 100644 --- a/spatialmath/base/transforms2d.py +++ b/spatialmath/base/transforms2d.py @@ -1336,7 +1336,6 @@ def trplot2( width: float = 1, d1: float = 0.1, d2: float = 1.15, - **kwargs, ): """ Plot a 2D coordinate frame @@ -1603,9 +1602,18 @@ def tranimate2(T: Union[SO2Array, SE2Array], **kwargs): """ dims = kwargs.pop("dims", None) ax = kwargs.pop("ax", None) + + # Animate2.run()'s own parameters must not also be forwarded to + # trplot2's drawing call, which now validates its keywords strictly + # (see Animate2.run's signature for this parameter set) + run_kwargs = { + k: kwargs.pop(k) + for k in ("movie", "repeat", "interval", "nframes", "wait") + if k in kwargs + } anim = smb.animate.Animate2(dims=dims, axes=ax, **kwargs) anim.trplot2(T, **kwargs) - return anim.run(**kwargs) + return anim.run(**run_kwargs) if __name__ == "__main__": # pragma: no cover diff --git a/spatialmath/base/transforms3d.py b/spatialmath/base/transforms3d.py index 11aba217..8f958e31 100644 --- a/spatialmath/base/transforms3d.py +++ b/spatialmath/base/transforms3d.py @@ -3029,7 +3029,6 @@ def trplot( dims: Optional[ArrayLikePure] = None, d2: float = 1.15, flo: Tuple[float, float, float] = (-0.05, -0.05, -0.05), - **kwargs, ): """ Plot a 3D coordinate frame @@ -3217,6 +3216,9 @@ def trplot( ax.set_proj_type("persp") # collect all the arguments to use for left and right views + # note: dims is deliberately excluded, it was already applied to + # ax above; block is deliberately excluded and handled once + # below, after both eyes are drawn, rather than per-eye args = { "ax": ax, "frame": frame, @@ -3225,8 +3227,15 @@ def trplot( "wtl": wtl, "flo": flo, "d2": d2, + "textcolor": textcolor, + "labels": labels, + "originsize": originsize, + "origincolor": origincolor, + "axislabel": axislabel, + "axissubscript": axissubscript, + "width": width, + "projection": projection, } - args = {**args, **kwargs} # unpack the anaglyph parameters shift = 0.1 @@ -3248,7 +3257,11 @@ def trplot( T = r2t(cast(SO3Array, T)) trplot(transl(shift, 0, 0) @ T, color=colors[1], **args) - return + if block is not None: + import matplotlib.pyplot as plt + + plt.show(block=block) + return ax if style == "rviz": if originsize is None: @@ -3300,9 +3313,9 @@ def trplot( flo=flo, anaglyph=anaglyph, axislabel=axislabel, - **kwargs, + axissubscript=axissubscript, ) - return + return ax if dims is not None: dims = tuple(dims) @@ -3516,9 +3529,18 @@ def tranimate(T: Union[SO3Array, SE3Array], **kwargs) -> str: # alias for backward compatibility with the previous docstring/API dim = kwargs.pop("dim", kwargs.pop("dims", None)) ax = kwargs.pop("ax", None) + + # Animate.run()'s own parameters must not also be forwarded to + # trplot's drawing call, which now validates its keywords strictly + # (see Animate.run's signature for this parameter set) + run_kwargs = { + k: kwargs.pop(k) + for k in ("movie", "repeat", "interval", "nframes", "wait") + if k in kwargs + } anim = Animate(dim=dim, ax=ax, **kwargs) anim.trplot(T, **kwargs) - return anim.run(**kwargs) + return anim.run(**run_kwargs) if __name__ == "__main__": # pragma: no cover diff --git a/tests/base/test_animate.py b/tests/base/test_animate.py new file mode 100644 index 00000000..147a09ad --- /dev/null +++ b/tests/base/test_animate.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +""" +Regression coverage for spatialmath.base.animate. + +animate()/tranimate()/tranimate2() build a matplotlib FuncAnimation, but +under a non-interactive backend with nothing pumping a real event loop +(exactly the CI/test environment -- see conftest.py, which forces the Agg +backend for the whole session) its per-frame update() callback never +actually fires. Merely constructing the FuncAnimation, or calling +.animate() and letting the result be garbage collected, proves nothing +about whether the interpolation code (trinterp/trinterp2) runs or is +correct -- it only proves construction didn't raise. + +FuncAnimation.save() is different: it synchronously steps through every +frame and invokes the real update() callback for each one, regardless of +backend -- the same thing a real playback or movie export would do. We +drive that here with PillowWriter (pure Python, ships with +matplotlib+Pillow, no external ffmpeg binary) purely to force every frame +through the real callback, and record what it actually computed, rather +than analyzing the saved output itself. +""" + +import os +import tempfile +import unittest +from unittest.mock import patch + +import matplotlib + +matplotlib.use("Agg") +import matplotlib.pyplot as plt +from matplotlib.animation import PillowWriter + +import spatialmath.base as smb +from spatialmath import SE2, SE3 + + +def _drive_all_frames(fa): + tmp = tempfile.mktemp(suffix=".gif") + try: + fa.save(tmp, writer=PillowWriter(fps=5)) + finally: + if os.path.exists(tmp): + os.remove(tmp) + + +class TestAnimate2(unittest.TestCase): + def test_drives_real_frames(self): + plt.close("all") + end = SE2.Rand() + start = SE2.Rand() + nframes = 5 + + real_trinterp2 = smb.trinterp2 + calls = [] + + def recording_trinterp2(*args, **kwargs): + calls.append(kwargs["s"]) + return real_trinterp2(*args, **kwargs) + + with patch("spatialmath.base.trinterp2", side_effect=recording_trinterp2): + fa = end.animate(start=start, dims=[-2, 2], nframes=nframes, repeat=False) + _drive_all_frames(fa) + + self.assertEqual(len(calls), nframes) + self.assertAlmostEqual(min(calls), 0.0) + self.assertAlmostEqual(max(calls), 1.0) + plt.close("all") + + +class TestAnimate3(unittest.TestCase): + def test_drives_real_frames(self): + plt.close("all") + end = SE3.Rand() + start = SE3.Rand() + nframes = 5 + + real_trinterp = smb.trinterp + calls = [] + + def recording_trinterp(*args, **kwargs): + calls.append(kwargs["s"]) + return real_trinterp(*args, **kwargs) + + with patch("spatialmath.base.trinterp", side_effect=recording_trinterp): + fa = end.animate(start=start, dims=[-2, 2], nframes=nframes, repeat=False) + _drive_all_frames(fa) + + self.assertEqual(len(calls), nframes) + self.assertAlmostEqual(min(calls), 0.0) + self.assertAlmostEqual(max(calls), 1.0) + plt.close("all") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/base/test_transforms2d.py b/tests/base/test_transforms2d.py index 6a406006..61e3adab 100755 --- a/tests/base/test_transforms2d.py +++ b/tests/base/test_transforms2d.py @@ -296,6 +296,14 @@ def test_plot(self): ) plt.close("all") + def test_plot_rejects_unknown_kwarg(self): + # trplot2() used to silently swallow an unrecognized keyword via a + # bare **kwargs sink instead of raising -- e.g. a typo'd + # framelabel= (the real parameter is frame=) drew the frame with + # no label and no error at all. + with self.assertRaises(TypeError): + trplot2(transl2(1, 2), framelabel="A", block=False) + # ---------------------------------------------------------------------------------------# if __name__ == "__main__": diff --git a/tests/base/test_transforms3d_plot.py b/tests/base/test_transforms3d_plot.py index d18d9300..2a4ba18e 100755 --- a/tests/base/test_transforms3d_plot.py +++ b/tests/base/test_transforms3d_plot.py @@ -73,6 +73,46 @@ def test_plot(self): plt.close("all") + def test_plot_rejects_unknown_kwarg(self): + # trplot() used to silently swallow an unrecognized keyword via a + # bare **kwargs sink instead of raising -- e.g. a typo'd + # framelabel= (the real parameter is frame=) drew the frame with + # no label and no error at all. + with self.assertRaises(TypeError): + trplot(transl(1, 2, 3), framelabel="A", block=False) + + def test_plot_anaglyph_forwards_params(self): + # the anaglyph branch recurses into trplot() for each eye; it used + # to forward only a small fixed subset of parameters (ax, frame, + # length, style, wtl, flo, d2), silently dropping textcolor, + # labels, originsize, origincolor, axislabel, axissubscript, width + # and projection for both eyes. + plt.figure() + ax = trplot( + transl(1, 2, 3), + frame="A", + anaglyph=True, + block=False, + textcolor="k", + originsize=0, + ) + # frame label + 3 axis labels, per eye + self.assertEqual(len(ax.texts), 8) + self.assertTrue(all(t.get_color() == "k" for t in ax.texts)) + plt.close("all") + + def test_plot_iterable_forwards_axissubscript(self): + # the "T is an iterable of transforms" branch used to forward + # every named parameter except axissubscript when recursing per + # transform, so axissubscript=False on the outer call was ignored. + plt.figure() + T = [transl(1, 2, 3), transl(2, 3, 4)] + ax = trplot(T, frame="F", axissubscript=False, block=False) + axis_labels = [t.get_text() for t in ax.texts if t.get_text() != r"$\{F\}$"] + self.assertTrue(axis_labels) + self.assertTrue(all("_{" not in text for text in axis_labels)) + plt.close("all") + @pytest.mark.skipif( plt.get_backend().lower() == "agg" or os.environ.get("CI") == "true" diff --git a/tests/test_pose2d.py b/tests/test_pose2d.py index fc3734d6..4e4f7151 100755 --- a/tests/test_pose2d.py +++ b/tests/test_pose2d.py @@ -509,7 +509,7 @@ def test_graphics(self): T1.plot(block=False, dims=[-2, 2]) T1.animate(repeat=False, dims=[-2, 2], nframes=10) - T1.animate(T0=T2, repeat=False, dims=[-2, 2], nframes=10) + T1.animate(start=T2, repeat=False, dims=[-2, 2], nframes=10) # ---------------------------------------------------------------------------------------#