Standardize coordinate schemes - #1
Conversation
Updates docstrings
Conventional is ISO right-handed
|
Docs will also need to be updated as they don't line up with what the coordinate systems previously were nor what this will make them. |
RealOrangeOne
left a comment
RealOrangeOne
left a comment
There was a problem hiding this comment.
I think this looks ok, although would appreciate someone more knowledgeable with it review the maths side.
| if os.environ.get('ZOLOTO_LEGACY_AXIS'): | ||
| return self.phi - (math.pi / 2) | ||
| else: | ||
| raise AttributeError("Rotation around this axis is not used") |
There was a problem hiding this comment.
Question: Why would this not be used?
There was a problem hiding this comment.
In this axis system this is a rotation that would be twist of the marker which is a property of the orientation, not the coordinate.
There was a problem hiding this comment.
Ah ok, I see. I think we should still improve the exception message though.
| initial_rotation.y, | ||
| ) * self.__MARKER_ORIENTATION_CORRECTION | ||
|
|
||
| if os.environ.get('ZOLOTO_LEGACY_AXIS'): |
There was a problem hiding this comment.
Suggestion: We should extract this as a constant
| A zero value is on the centerline of the image. | ||
| """ | ||
| if os.environ.get('ZOLOTO_LEGACY_AXIS'): | ||
| return -self.theta |
There was a problem hiding this comment.
Issue: A lot of these "new" legacy axis codepaths don't have tests.
That said, I'm slightly concerned the tests aren't failing in their current state...
There was a problem hiding this comment.
Since this is merging into Peter's fork the tests aren't being run.
There was a problem hiding this comment.
Ah, that would do it! Do the tests pass locally?
Co-authored-by: Dan Trickey <trickeydan@users.noreply.github.com>
3 participants
Moves to using the standard right-handed coordinate system and spherical coordinates in mathematical notation.
Yaw, pitch and roll are also corrected to their expected directions.
Setting the environment variable ZOLOTO_LEGACY_AXIS uses the axis as in version 0.9.0, except for yaw, pitch and roll which always follows their expected directions.
Builds upon RealOrangeOne#302.