-
Notifications
You must be signed in to change notification settings - Fork 37
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
Adds Pose equivalency functions #130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Have another batch of comments.
Also, once this in place, you could consider going through some of the code/unit tests that are checking pose equality the old way and making the updates. Not necessary, though, only if you want to!
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoshuaFutcher I found 2 bugs and fixed them on main
.
This would have been caught if we'd had #97 implemented -- sorry!
Just sharing them here for your reference.
@@ -199,6 +208,44 @@ def __repr__(self): | |||
) | |||
return f"Pose: [{pos_str}, {quat_str}]" | |||
|
|||
def is_approx(self, other, rel_tol=1e-09, abs_tol=0.0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the default abs_tol=0
, then is_approx()
will always return False for non-identical poses with default values. I changed these both to default to 1e-6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, I misunderstood how the abs_tol works then : ) thanks
raise TypeError("Expected a Pose") | ||
|
||
return np.allclose( | ||
self.get_translation, other.get_translation, rel_tol, abs_tol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're supposed to actually call the function
self.get_translation, other.get_translation, rel_tol, abs_tol | |
self.get_translation(), other.get_translation(), rel_tol, abs_tol |
if not (isinstance(other, Pose)): | ||
raise TypeError("Expected a Pose") | ||
|
||
return np.all(self.get_translation == other.get_translation) and np.all( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and same here
return np.all(self.get_translation == other.get_translation) and np.all( | |
return np.all(self.get_translation() == other.get_translation()) and np.all( |
Fixes #127 - Weird issue with rel_tol=1e-09 being Pose instead of float?