From e7b6edad0a291591afd1f9a0b98715e41e1866a7 Mon Sep 17 00:00:00 2001 From: Brian Okorn Date: Thu, 4 Jan 2024 16:20:39 -0500 Subject: [PATCH 1/5] Fixed fall through error in binary operation --- spatialmath/baseposematrix.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spatialmath/baseposematrix.py b/spatialmath/baseposematrix.py index da1421e9..60c0536b 100644 --- a/spatialmath/baseposematrix.py +++ b/spatialmath/baseposematrix.py @@ -1683,7 +1683,8 @@ def _op2(left, right: Self, op: Callable): # pylint: disable=no-self-argument return op(left.A, right) else: return [op(x, right) for x in left.A] - + else: + raise ValueError(f'Invalid type ({right.__class__}) for binary operation with {left.__class__}') if __name__ == "__main__": from spatialmath import SE3, SE2, SO2 From 5c432b3f3f8b72e6a90f7ffc65e8c6799862e0ba Mon Sep 17 00:00:00 2001 From: Jien Cao Date: Fri, 5 Jan 2024 09:57:36 -0500 Subject: [PATCH 2/5] add unit test coverage --- tests/test_pose3d.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_pose3d.py b/tests/test_pose3d.py index 04ed1e12..2da2d5d0 100755 --- a/tests/test_pose3d.py +++ b/tests/test_pose3d.py @@ -1260,6 +1260,35 @@ def test_arith_vect(self): array_compare(a[1], ry - 1) array_compare(a[2], rz - 1) + def test_angle(self): + # angle between SO3's + r1 = SO3.Rx(0.1) + r2 = SO3.Rx(0.2) + for metric in range(6): + self.assertAlmostEqual(r1.angdist(other=r1, metric=metric), 0.0) + self.assertGreater(r1.angdist(other=r2, metric=metric), 0.0) + self.assertAlmostEqual( + r1.angdist(other=r2, metric=metric), r2.angdist(other=r1, metric=metric) + ) + # angle between SE3's + p1a, p1b = SE3.Rx(0.1), SE3.Rx(0.1, t=(1, 2, 3)) + p2a, p2b = SE3.Rx(0.2), SE3.Rx(0.2, t=(3, 2, 1)) + for metric in range(6): + self.assertAlmostEqual(p1a.angdist(other=p1a, metric=metric), 0.0) + self.assertGreater(p1a.angdist(other=p2a, metric=metric), 0.0) + self.assertAlmostEqual(p1a.angdist(other=p1b, metric=metric), 0.0) + self.assertAlmostEqual( + p1a.angdist(other=p2a, metric=metric), + p2a.angdist(other=p1a, metric=metric), + ) + self.assertAlmostEqual( + p1a.angdist(other=p2a, metric=metric), + p1a.angdist(other=p2b, metric=metric), + ) + # angle between mismatched types + with self.assertRaises(ValueError): + _ = r1.angdist(p1a) + def test_functions(self): # inv # .T From 336fbc8844754775958c516969a0ae470a441a0a Mon Sep 17 00:00:00 2001 From: Jien Cao Date: Fri, 5 Jan 2024 10:54:36 -0500 Subject: [PATCH 3/5] update test + type check --- spatialmath/baseposematrix.py | 7 +++++-- tests/test_pose3d.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/spatialmath/baseposematrix.py b/spatialmath/baseposematrix.py index 60c0536b..221bb6fd 100644 --- a/spatialmath/baseposematrix.py +++ b/spatialmath/baseposematrix.py @@ -1657,7 +1657,7 @@ def _op2(left, right: Self, op: Callable): # pylint: disable=no-self-argument ========= ========== ==== ================================ """ - if isinstance(right, left.__class__): + if right.__class__ is left.__class__: # class by class if len(left) == 1: if len(right) == 1: @@ -1684,7 +1684,10 @@ def _op2(left, right: Self, op: Callable): # pylint: disable=no-self-argument else: return [op(x, right) for x in left.A] else: - raise ValueError(f'Invalid type ({right.__class__}) for binary operation with {left.__class__}') + raise TypeError( + f"Invalid type ({right.__class__}) for binary operation with {left.__class__}" + ) + if __name__ == "__main__": from spatialmath import SE3, SE2, SO2 diff --git a/tests/test_pose3d.py b/tests/test_pose3d.py index 2da2d5d0..e8728e6a 100755 --- a/tests/test_pose3d.py +++ b/tests/test_pose3d.py @@ -1285,10 +1285,17 @@ def test_angle(self): p1a.angdist(other=p2a, metric=metric), p1a.angdist(other=p2b, metric=metric), ) - # angle between mismatched types - with self.assertRaises(ValueError): + # not allowing angdist between mismatched types + with self.assertRaises(TypeError): _ = r1.angdist(p1a) + # in general, the _op2 interface allows same type only + with self.assertRaises(TypeError): + _ = r1._op2(right=p1a, op=r1.angdist) + + with self.assertRaises(TypeError): + _ = p1a._op2(right=r1, op=p1a.angdist) + def test_functions(self): # inv # .T From b848e7bff7448d16af5b0c7fd3d5498c515cb7cf Mon Sep 17 00:00:00 2001 From: Jien Cao Date: Wed, 10 Jan 2024 13:11:00 -0500 Subject: [PATCH 4/5] updates based on review comments --- spatialmath/baseposematrix.py | 2 +- tests/test_pose3d.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spatialmath/baseposematrix.py b/spatialmath/baseposematrix.py index 221bb6fd..379a5439 100644 --- a/spatialmath/baseposematrix.py +++ b/spatialmath/baseposematrix.py @@ -1657,7 +1657,7 @@ def _op2(left, right: Self, op: Callable): # pylint: disable=no-self-argument ========= ========== ==== ================================ """ - if right.__class__ is left.__class__: + if isinstance(right, left.__class__) or isinstance(left, right.__class__): # class by class if len(left) == 1: if len(right) == 1: diff --git a/tests/test_pose3d.py b/tests/test_pose3d.py index e8728e6a..52d889ce 100755 --- a/tests/test_pose3d.py +++ b/tests/test_pose3d.py @@ -1286,14 +1286,14 @@ def test_angle(self): p1a.angdist(other=p2b, metric=metric), ) # not allowing angdist between mismatched types - with self.assertRaises(TypeError): + with self.assertRaises(ValueError): _ = r1.angdist(p1a) # in general, the _op2 interface allows same type only - with self.assertRaises(TypeError): + with self.assertRaises(ValueError): _ = r1._op2(right=p1a, op=r1.angdist) - with self.assertRaises(TypeError): + with self.assertRaises(ValueError): _ = p1a._op2(right=r1, op=p1a.angdist) def test_functions(self): From c234e6c7a2ad9591c4f3ce81f7b0ef03944dded6 Mon Sep 17 00:00:00 2001 From: Jien Cao Date: Wed, 10 Jan 2024 13:25:19 -0500 Subject: [PATCH 5/5] one more test --- tests/test_pose3d.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_pose3d.py b/tests/test_pose3d.py index 52d889ce..7132bddc 100755 --- a/tests/test_pose3d.py +++ b/tests/test_pose3d.py @@ -1285,17 +1285,20 @@ def test_angle(self): p1a.angdist(other=p2a, metric=metric), p1a.angdist(other=p2b, metric=metric), ) - # not allowing angdist between mismatched types + # angdist is not implemented for mismatched types with self.assertRaises(ValueError): _ = r1.angdist(p1a) - # in general, the _op2 interface allows same type only with self.assertRaises(ValueError): _ = r1._op2(right=p1a, op=r1.angdist) with self.assertRaises(ValueError): _ = p1a._op2(right=r1, op=p1a.angdist) + # in general, the _op2 interface enforces an isinstance check. + with self.assertRaises(TypeError): + _ = r1._op2(right=(1, 0, 0), op=r1.angdist) + def test_functions(self): # inv # .T