-
Notifications
You must be signed in to change notification settings - Fork 1
Add marker orientations #34
base: master
Are you sure you want to change the base?
Changes from 10 commits
50d38c5
b711cbb
d17aed4
217b7a3
a34fd37
f69d719
db590b6
0e066c8
843e455
361ce09
4a79783
5a8a949
638580b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| """Tests for marker orientations.""" | ||
|
|
||
| import math | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
| from pytest import approx | ||
|
|
||
| from sb_vision import FileCamera, Orientation, Vision | ||
|
|
||
| ROTATION_TOLERANCE_DEGREES = 6 | ||
|
|
||
| CALIBRATIONS = Path(__file__).parent.parent / 'calibrations' / 'tecknet_rotations' | ||
|
|
||
| TEST_IMAGES = [ | ||
| ( | ||
| '0rot_x-45rot_y1.1z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=math.radians(-45), rot_z=0), | ||
| ), | ||
| ( | ||
| '-45rot_x0rot_y1.1z.png', | ||
| False, | ||
| Orientation(rot_x=math.radians(-45), rot_y=0, rot_z=0), | ||
| ), | ||
| ( | ||
| '-22.5rot_x0rot_y0.6z.png', | ||
| False, | ||
| Orientation(rot_x=math.radians(-22.5), rot_y=0, rot_z=0), | ||
| ), | ||
| ( | ||
| '0rot_x-22.5rot_y0.6z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=math.radians(-22.5), rot_z=0), | ||
| ), | ||
| ( | ||
| '0rot_x0rot_y0.55z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=0, rot_z=0), | ||
| ), | ||
| ( | ||
| '-90rot_z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=0, rot_z=math.radians(270)), | ||
| ), | ||
| ( | ||
| '90rot_z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=0, rot_z=math.radians(90)), | ||
| ), | ||
| ( | ||
| '180rot_z.png', | ||
| True, | ||
| Orientation(rot_x=0, rot_y=0, rot_z=math.radians(180)), | ||
| ), | ||
| ( | ||
| '135rot_z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=0, rot_z=math.radians(135)), | ||
| ), | ||
| ( | ||
| '45rot_z.png', | ||
| False, | ||
| Orientation(rot_x=0, rot_y=0, rot_z=math.radians(45)), | ||
| ), | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "photo, allow_wrapping, expected_orientation", | ||
| TEST_IMAGES, | ||
| ) | ||
| def test_image_coordinates(photo, allow_wrapping, expected_orientation): | ||
| """Make sure that this particular file gives these particular tokens.""" | ||
| camera = FileCamera(CALIBRATIONS / photo, camera_model='C016') | ||
| vision = Vision(camera) | ||
| token, = vision.snapshot() | ||
|
|
||
| def approx_ang(expected_degrees): | ||
| return approx(expected_degrees, abs=ROTATION_TOLERANCE_DEGREES) | ||
|
|
||
| def assert_angle(angle_radians, expected_degrees, message): | ||
| expected_degrees = approx_ang(math.degrees(expected_degrees)) | ||
| # Check both +0 and +360 so approx can cover the jump between -180 and 180 | ||
| if allow_wrapping: | ||
| assert \ | ||
| math.degrees(angle_radians) == expected_degrees or \ | ||
| math.degrees(angle_radians) + 360 == expected_degrees, \ | ||
| message | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to do this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we can allow for a small error on a value which is close to 180 or -180. it means it checks both the 180 version and the -180 version in one check.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but these are tests -- they're dealing with fixed inputs which should mean that they have fixed outputs. We should know which side the result is and just assert that. Are you saying that we get varying results from An alternative way of spelling this, iff it's absolutely needed, might be to declare that the tests operate in a positive-only frame and run all the values through assert -180 < actual_degrees < 180, \
"{} is outside the expected range (-180° to 180°)".format(angle_name)
# angle_name is one of rot_x, rot_y, etc.
# Bound to positive numbers for easier comparisons
actual_degrees %= 360
assert actual_degrees == expected_degrees, ...While I'll admit there's a bit more processing here, degrees being a modulo thing is fairly obvious to everyone since we're used to the idea that 720° is the direction same as 0°.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I respect what you're saying but I disagree that we should check exact values. It makes the tests extremely fragile for changes higher up the pipeline (i.e. thresholding). If we want to check that the outputs don't vary, we should have a separate test
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| else: | ||
| assert math.degrees(angle_radians) == expected_degrees, message | ||
|
|
||
| rot_x, rot_y, rot_z = token.orientation | ||
|
|
||
| assert_angle(rot_x, expected_orientation.rot_x, "Wrong Orientation rot_x") | ||
| assert_angle(rot_y, expected_orientation.rot_y, "Wrong Orientation rot_y") | ||
| assert_angle(rot_z, expected_orientation.rot_z, "Wrong Orientation rot_z") | ||
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.
Conditionals in tests are a bad idea; they usually indicate that the test is trying to test too many things and should be broken apart. Tests should be so simple that they are obviously correct.