Skip to content
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

Fix Plane move and add moved #444

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

MatthiasJ1
Copy link
Contributor

Plane.move contradicts the documentation on move vs moved (here) and is missing moved, therefore operations like Pos() * Plane will fail.

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb5625c) 96.04% compared to head (45c04d4) 96.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #444      +/-   ##
==========================================
- Coverage   96.04%   96.00%   -0.04%     
==========================================
  Files          24       24              
  Lines        7425     7433       +8     
==========================================
+ Hits         7131     7136       +5     
- Misses        294      297       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gumyr
Copy link
Owner

gumyr commented Jan 7, 2024

Thank you! The test for Plane.move isn't complete:

    def test_move(self):
        pln = Plane.XY.move(Location((1, 2, 3)))
        self.assertVectorAlmostEquals(pln.origin, (1, 2, 3), 5)

Would you correct this test by checking that the instance of the Plane hasn't changed and introduce a new test for moved that checks that the instances are different?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants