search

bdaiinstitute / spatialmath-python

Create, manipulate and convert representations of position and orientation in 2D or 3D using Python
MIT License
517 stars 85 forks source link

Fall through error in the binary operator for BasePoseMatrix #116

Closed bokorn-bdaii closed 9 months ago

bokorn-bdaii commented 10 months ago

Fall through error in the binary operator for BasePoseMatrix:

https://github.com/bdaiinstitute/spatialmath-python/blob/1b89c49395a21b5241e2f0a233e69394f3bc27b1/spatialmath/baseposematrix.py#L1632C7-L1632C7

If you want to compare the rotation of an SE3 to an SO3, it returns none as opposed to throwing an error or returning a valid value. This should probably raise an error as opposed to returning None. 

import numpy as np
from spatialmath import SE3, SO3

T0 = SE3()
print(T0.angdist(SO3()))

Could be fixed with closing else in BasePoseMatrix._op2

    else:
        raise ValueError(f'Invalid type ({right.__class__}) for binary operation with {left.__class__}')
jcao-bdai commented 10 months ago

Thank you for noticing this, @bokorn-bdaii

i see you have a PR up with this fix but it also exposed the fact that we did not have test coverage for this logic - do you mind if I push a commit with just a unit test onto your branch?

bokorn-bdaii commented 10 months ago

Ya, of course. Feel free to push a unit test in.

jcao-bdai commented 10 months ago

@bokorn-bdaii pushed. thanks! :crossed_fingers: will approve as soon as the workflow completes. thanks again for fixing this issue.

jcao-bdai commented 9 months ago

@bokorn-bdaii your PR fixing this issue has been merged: https://github.com/bdaiinstitute/spatialmath-python/pull/117