[Maintenance] Refactor issue 113 - offset-rotation procedure to _transformations.py#166
[Maintenance] Refactor issue 113 - offset-rotation procedure to _transformations.py#16620086080 wants to merge 8 commits intocamUrban:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
=======================================
Coverage 91.71% 91.71%
=======================================
Files 35 35
Lines 6828 6830 +2
=======================================
+ Hits 6262 6264 +2
Misses 566 566 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I think this change was committed accidentally. You might want to rename your PyCharm project to "PteraSoftware" to match the project default, or add this file to your personal .git/info/exclude file.
There was a problem hiding this comment.
I think this change was committed accidentally. You might want to rename your PyCharm project to "PteraSoftware" to match the project default, or add this file to your personal .git/info/exclude file.
There was a problem hiding this comment.
I think this change was committed accidentally. You might want to add this file to your personal .git/info/exclude file.
| :return: A (3,) ndarray representing the position adjustment. | ||
| """ | ||
|
|
||
| return (np.eye(3, dtype=float) - rotation_matrix) @ offset |
There was a problem hiding this comment.
I think you can fix the mypy warning here by wrapping this in np.asarray(..., dtype=float). Then, double-check you don't get any warnings by running mypy pterasoftware.
| When a rotation is applied about a point other than the origin, the effective | ||
| position must be adjusted. This function computes that adjustment vector. | ||
|
|
||
| The adjustment is given by: (I - R) @ offset |
There was a problem hiding this comment.
You can drop the extra white space after the colon here.
| npt.assert_allclose(adjustment, np.zeros(3), atol=1e-14) | ||
|
|
||
| def test_known_rotation(self): | ||
| """Test adjustment for a known rotation (90° about z-axis).""" |
There was a problem hiding this comment.
Replace with "90 deg" (we try to avoid any characters that aren't in the set of 95 printable-ASCII characters).
|
|
||
|
|
||
| class TestComputeOffsetRotationAdjustment(unittest.TestCase): | ||
| """Tests for compute_offset_rotation_adjustment function.""" |
There was a problem hiding this comment.
Add "the" to "Tests for compute_..."
camUrban
added
the
maintenance
|
Also, don't forget to add yourself to the list of contributors in the README! 😄 |
|
@camUrban Hi. Thank you for all the comments and suggestions. I have made the requested changes. Let me know if anything additional on this. Thank you. |
2 participants
Description
Refactor rotation handling in
CoreWingMovementand add targeted unit tests to validate the rotation-point offset transformation logic.This improves clarity and correctness of how offset-based rotations are applied, and introduces focused tests to validate transformation behavior in a controlled and reproducible way. The rotation handling logic can now be reused elsewhere as required.
I am a a first-time contributor and will add myself to the list of contributors in the README. Thank you.
Motivation
The rotation-point offset logic in
WingMovement(though actually in the_coresuper class ingenerate_wing_at_time_step method) is mathematically non-trivial and previously untested in isolation. This makes it a difficult task to confidently refactor or extend and has a level of complexity which I took on as a challenge.Additionally, the transformation logic relies on
_transformations.generate_rot_T, so ensuring correctness requires validating both:WingMovement.Relevant Issues
Closes #113.
Changes
Refactor
WingMovement.generate_wing_at_time_step.New Tests
Added unit tests in
test_transformations.pyto validate transformation behavior.test_identity_rotation- Identity rotation should produce zero adjustmenttest_zero_offset- Zero offset should always produce zero adjustmenttest_known_rotation- Test adjustment for a known rotation (90° about z-axis)Introduced tests (class
TestComputeOffsetRotationAdjustment) specifically targeting:generate_rot_T.(I - R) @ offsetproduces expected translation.Used
numpy.testing.assert_allclosewith tight tolerances(atol=1e-14)to ensure numerical precision.Testing Methodology
Validation
Dependency Updates
None.
Change Magnitude
Minor: Small refactor with added tests to improve scalability, correctness and maintainability.
Checklist (check each item when completed or not applicable)
mainbranch.--in-place --black). See the style guide for type hints and docstrings for more details.pterasoftwarepackage use type hints. See the style guide for type hints and docstrings for more details.testspackage.testspackage.black,codespell, andisortGitHub actions.mypyGitHub action.testsGitHub actions.