RealOrangeOne / zoloto

A fiducial marker system powered by OpenCV - Supports ArUco and April
https://zoloto.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
13 stars 7 forks source link

`Orientation.__repr__` is invalid #309

Open PeterJCLaw opened 2 years ago

PeterJCLaw commented 2 years ago

An object's repr is intended to be either a valid Python construct to rebuild an equivalent object or something enclosed in angle brackets. Orientation.__repr__ matches neither of these.

Notably the test_repr which exists for the type fails to actually validate this, though it could with a small extension:

@given(tuples(floats(), floats(), floats()))
def test_repr(euler_angles: Tuple[float, float, float]) -> None:
    """Test that the representation is as expected."""
    q = Quaternion(axis=euler_angles, scalar=1)
    orientation = Orientation(*q.vector)

    ypr = q.yaw_pitch_roll
    names = ["rot_z", "rot_y", "rot_x"]

    repr_str = repr(orientation)

    for name, val in zip(names, ypr):
        assert "{}={}".format(name, val) in repr_str

    rebuilt = Orientation(**dict(zip(names, ypr)))
    assert repr(rebuilt) == repr(orientation), "Repr value should round-trip"
diff --git a/tests/test_coords.py b/tests/test_coords.py
index d16f7f1..8790482 100644
--- a/tests/test_coords.py
+++ b/tests/test_coords.py
@@ -62,3 +62,7 @@ def test_repr(euler_angles: Tuple[float, float, float]) -> None:

     for name, val in zip(names, ypr):
         assert "{}={}".format(name, val) in repr_str
+
+    rebuilt = Orientation(**dict(zip(names, ypr)))
+
+    assert repr(rebuilt) == repr(orientation)
PeterJCLaw commented 2 years ago

Hrm, interestingly though this property does hold:

    rebuilt = Orientation(*orientation.yaw_pitch_roll)
    assert repr(rebuilt) == repr(orientation)

So it looks like it's just the names which are out of sync.

PeterJCLaw commented 2 years ago

Or not. Using the code from that test I'm struggling to come up with input values that result in a Quaternion other than Quaternion(1.0, 0.0, 0.0, 0.0), which of course always results in a zero orientation. Unsurprisingly zero will always round trip.