-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor euler and quaternion conversion functions #9
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
Conversation
Updated euler_to_quat and quat_to_euler functions to include degrees parameter and simplified conversion logic.
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9 +/- ##
==========================================
- Coverage 90.63% 90.54% -0.09%
==========================================
Files 6 6
Lines 427 423 -4
Branches 53 53
==========================================
- Hits 387 383 -4
Misses 26 26
Partials 14 14
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR refactors the euler and quaternion conversion functions to simplify the implementation and add support for degree/radian conversion. The changes remove unnecessary intermediate steps through rotation matrices and add a degrees parameter to both functions.
- Simplified
euler_to_quatandquat_to_eulerfunctions by removing intermediate rotation matrix conversions - Added
degreesparameter to both functions for angle unit specification - Updated test assertions to reflect the corrected conversion behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vortex_utils/python_utils.py | Refactored conversion functions with direct scipy.spatial.transform.Rotation calls and added degrees parameter |
| tests/test_utils.py | Updated test expectations to match corrected conversion results |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| roll: float, pitch: float, yaw: float, *, degrees: bool = False | ||
| ) -> np.ndarray: | ||
| """RPY (intrinsic x-y-z) -> quaternion (x, y, z, w).""" | ||
| return Rotation.from_euler('xyz', [roll, pitch, yaw], degrees=degrees).as_quat() |
Copilot
AI
Sep 30, 2025
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.
The euler sequence changed from 'XYZ' (extrinsic) to 'xyz' (intrinsic) without clear documentation of this breaking change. This affects the rotation order and could break existing code that depends on the previous behavior.
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.
nobody uses my code anyway
| x: float, y: float, z: float, w: float, *, degrees: bool = False | ||
| ) -> np.ndarray: | ||
| """Quaternion (x, y, z, w) -> RPY (intrinsic x-y-z).""" | ||
| return Rotation.from_quat([x, y, z, w]).as_euler('xyz', degrees=degrees) |
Copilot
AI
Sep 30, 2025
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.
The euler sequence changed from 'XYZ' (extrinsic) to 'xyz' (intrinsic) without clear documentation of this breaking change. This affects the rotation order and could break existing code that depends on the previous behavior.
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.
nobody uses my code anyway
kluge7
left a comment
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.
🚶♂️
|
🎉 This PR is included in version 1.4.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Updated euler_to_quat and quat_to_euler functions to include degrees parameter and simplified conversion logic.