From babe57a7a8129b58bfdf3017f98a5ecbf126c9aa Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Thu, 20 Aug 2026 20:56:08 +1000 Subject: [PATCH] deprecate: UnitQuaternion.interp1's shortest parameter is a no-op Every UnitQuaternion has a non-negative scalar part by construction (qunit()), and interp1 always interpolates from the identity quaternion, whose dot product with any unit quaternion is just that quaternion's own scalar part. The negative-dot-product condition `shortest` checks for to detect "the long way round" can therefore never occur here -- confirmed empirically (2000 random compositions, plus explicit UnitQuaternion([-1,0,0,0]) construction, all canonicalize to non-negative scalar part). Drops the dead branch, warns on shortest=True, documents why in a docstring note, and pins the no-op behavior (identical results for True/False, warns only when True) with a regression test. --- spatialmath/quaternion.py | 29 +++++++++++++++++++++++++---- tests/test_quaternion.py | 26 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/spatialmath/quaternion.py b/spatialmath/quaternion.py index a955241e..858522e8 100644 --- a/spatialmath/quaternion.py +++ b/spatialmath/quaternion.py @@ -16,6 +16,7 @@ # pylint: disable=invalid-name from __future__ import annotations import math +import warnings import numpy as np from typing import Any import spatialmath.base as smb @@ -2073,7 +2074,7 @@ def interp1(self, s: float = 0, shortest: Optional[bool] = False) -> UnitQuatern """ Interpolate a unit quaternion - :param shortest: Take the shortest path along the great circle + :param shortest: deprecated, has no effect :param s: interpolation coefficient, range 0 to 1, or number of steps :type s: array_like or int :return: interpolated unit quaternion @@ -2104,19 +2105,39 @@ def interp1(self, s: float = 0, shortest: Optional[bool] = False) -> UnitQuatern .. note:: values of ``s`` are silently clipped to the range [0, 1] + .. note:: ``shortest`` can never change the result here. Every + ``UnitQuaternion`` has a non-negative scalar part by construction + (see :func:`~spatialmath.base.quaternions.qunit`), and this method + always interpolates *from* the identity quaternion ``[1,0,0,0]`` + -- whose dot product with any unit quaternion is just that + quaternion's own (always non-negative) scalar part. A negative + dot product is what ``shortest`` checks for to detect "the long + way round", and that condition can never occur here, unlike + :meth:`interp`, where it can. + + .. deprecated:: 1.1.19 + ``shortest`` has no effect and will be removed in a future + release. + :seealso: :func:`~spatialmath.base.quaternions.qslerp` """ # TODO allow self to have len() > 1 + if shortest: + warnings.warn( + "shortest has no effect on interp1 and will be removed in a " + "future release: interpolation from the identity quaternion " + "is always the shortest path", + DeprecationWarning, + ) + if isinstance(s, int) and s > 1: s = np.linspace(0, 1, s) else: s = smb.getvector(s) s = np.clip(s, 0, 1) # enforce valid values - q0_endpoint, q0, q1, sin_theta, theta = _qslerp_prepare( - smb.qeye(), self.vec, shortest=shortest - ) + q0_endpoint, q0, q1, sin_theta, theta = _qslerp_prepare(smb.qeye(), self.vec) qi = [] for sk in s: if sk == 0: diff --git a/tests/test_quaternion.py b/tests/test_quaternion.py index 8e291573..cc08e4e6 100644 --- a/tests/test_quaternion.py +++ b/tests/test_quaternion.py @@ -3,6 +3,7 @@ import numpy.testing as nt import unittest from unittest.mock import patch +import warnings from spatialmath import * from spatialmath.base import * @@ -783,6 +784,31 @@ def test_interp_prepares_slerp_once(self): self.assertEqual(len(interpolate()), 5) self.assertEqual(dot.call_count, 1) + def test_interp1_shortest_deprecated(self): + # shortest never had an effect on interp1: every UnitQuaternion has + # a non-negative scalar part by construction (qunit()), and interp1 + # always interpolates from the identity quaternion, whose dot + # product with any unit quaternion is just that quaternion's own + # (always non-negative) scalar part. So the "long way round" branch + # shortest exists to avoid is unreachable. Pin that True/False give + # identical results, that it warns, and that leaving it at its + # default doesn't. + q = UnitQuaternion.Rx(4.5) # would be "the long way" if reachable + + for s in (0, 0.25, 0.5, 0.75, 1): + qcompare(q.interp1(s, shortest=False), q.interp1(s, shortest=True)) + + for a, b in zip(q.interp1(11, shortest=False), q.interp1(11, shortest=True)): + qcompare(a, b) + + with self.assertWarns(DeprecationWarning): + q.interp1(0.5, shortest=True) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + q.interp1(0.5) # default shortest=False must not warn + q.interp1(0.5, shortest=False) + def test_increment(self): q = UnitQuaternion()