Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fall through error in the binary operator for BasePoseMatrix #116

Closed
bokorn-bdaii opened this issue Jan 4, 2024 · 4 comments
Closed

Fall through error in the binary operator for BasePoseMatrix #116

bokorn-bdaii opened this issue Jan 4, 2024 · 4 comments

Comments

@bokorn-bdaii
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Contributor Author

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

@jcao-bdai
Copy link
Collaborator

jcao-bdai commented Jan 5, 2024

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

@jcao-bdai jcao-bdai reopened this Jan 10, 2024
@jcao-bdai
Copy link
Collaborator

@bokorn-bdaii your PR fixing this issue has been merged: #117

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

No branches or pull requests

3 participants