Adds Pose equivalency functions #130
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
July 25, 2023 09:22
sea-bass
reviewed
Jul 25, 2023
sea-bass
reviewed
Jul 25, 2023
sea-bass
reviewed
Jul 25, 2023
sea-bass
reviewed
Jul 25, 2023
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
sea-bass
approved these changes
Jul 25, 2023
sea-bass
reviewed
Jul 26, 2023
There was a problem hiding this comment.
@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.
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.
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.
we're supposed to actually call the function
Suggested change
| 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.
and same here
Suggested change
| return np.all(self.get_translation == other.get_translation) and np.all( | |
| return np.all(self.get_translation() == other.get_translation()) and np.all( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #127 - Weird issue with rel_tol=1e-09 being Pose instead of float?