From f32e4b1885f36c2ca63006b4045c54114d675023 Mon Sep 17 00:00:00 2001 From: beverly Date: Mon, 15 Mar 2021 08:28:15 +0100 Subject: [PATCH 1/4] fix custom magic methods and add tests --- CHANGELOG.md | 2 + src/compas/geometry/primitives/frame.py | 2 + src/compas/geometry/primitives/polygon.py | 7 ++- src/compas/geometry/primitives/polyline.py | 7 ++- src/compas/geometry/primitives/quaternion.py | 5 ++ tests/compas/geometry/test_polygon.py | 55 ++++++++++++++++++++ tests/compas/geometry/test_polyline.py | 55 ++++++++++++++++++-- 7 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 tests/compas/geometry/test_polygon.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a62e35ae59f..7bd3a253d148 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Fixed bug where mimic joints were considered configurable. * Fixed bug where `!=` gave incorrect results in Rhino for some compas objects. * Fixed bug where `compas_rhino.BaseArtist.redraw` did not trigger a redraw. +* Fixed minor bugs in `compas.geometry.Polyline` and `compas.geometry.Polygon`. +* Fixed very minor bugs in `compas.geometry.Frame` and `compas.geometry.Quaternion`. ### Removed diff --git a/src/compas/geometry/primitives/frame.py b/src/compas/geometry/primitives/frame.py index 71d2fec48f69..8098fa51fc27 100644 --- a/src/compas/geometry/primitives/frame.py +++ b/src/compas/geometry/primitives/frame.py @@ -180,6 +180,8 @@ def __iter__(self): return iter([self.point, self.xaxis, self.yaxis]) def __eq__(self, other, tol=1e-05): + if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): + return False for v1, v2 in zip(self, other): for a, b in zip(v1, v2): if math.fabs(a - b) > tol: diff --git a/src/compas/geometry/primitives/polygon.py b/src/compas/geometry/primitives/polygon.py index 1422a137cd8b..15752418d45f 100644 --- a/src/compas/geometry/primitives/polygon.py +++ b/src/compas/geometry/primitives/polygon.py @@ -147,7 +147,7 @@ def area(self): # ========================================================================== def __repr__(self): - return "Polygon({})".format(", ".join(["{}".format(point) for point in self.points])) + return "Polygon([{}])".format(", ".join(["{}".format(point) for point in self.points])) def __len__(self): return len(self.points) @@ -156,12 +156,15 @@ def __getitem__(self, key): return self.points[key] def __setitem__(self, key, value): - self.points[key] = value + self.points[key] = Point(*value) + self._lines = None def __iter__(self): return iter(self.points) def __eq__(self, other): + if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): + return False return all(a == b for a, b in zip(self, other)) # ========================================================================== diff --git a/src/compas/geometry/primitives/polyline.py b/src/compas/geometry/primitives/polyline.py index 726bc2d01447..c1cf4c2c3ffd 100644 --- a/src/compas/geometry/primitives/polyline.py +++ b/src/compas/geometry/primitives/polyline.py @@ -109,7 +109,7 @@ def length(self): # ========================================================================== def __repr__(self): - return "Polyline({})".format(", ".join(["{}".format(point) for point in self.points])) + return "Polyline([{}])".format(", ".join(["{}".format(point) for point in self.points])) def __len__(self): return len(self.points) @@ -118,12 +118,15 @@ def __getitem__(self, key): return self.points[key] def __setitem__(self, key, value): - self.points[key] = value + self.points[key] = Point(*value) + self._lines = None def __iter__(self): return iter(self.points) def __eq__(self, other): + if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): + return False return all(a == b for a, b in zip(self, other)) # ========================================================================== diff --git a/src/compas/geometry/primitives/quaternion.py b/src/compas/geometry/primitives/quaternion.py index e00255c974c0..0ab136f0674d 100644 --- a/src/compas/geometry/primitives/quaternion.py +++ b/src/compas/geometry/primitives/quaternion.py @@ -217,6 +217,8 @@ def __setitem__(self, key, value): raise KeyError def __eq__(self, other, tol=1e-05): + if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): + return False for v1, v2 in zip(self, other): if math.fabs(v1 - v2) > tol: return False @@ -225,6 +227,9 @@ def __eq__(self, other, tol=1e-05): def __iter__(self): return iter(self.wxyz) + def __len__(self): + return 4 + def __repr__(self): return 'Quaternion({:.{prec}f}, {:.{prec}f}, {:.{prec}f}, {:.{prec}f})'.format(self.w, self.x, self.y, self.z, prec=3) diff --git a/tests/compas/geometry/test_polygon.py b/tests/compas/geometry/test_polygon.py new file mode 100644 index 000000000000..a30d595d71dc --- /dev/null +++ b/tests/compas/geometry/test_polygon.py @@ -0,0 +1,55 @@ +import pytest + +from compas.geometry import Point +from compas.geometry import Polygon +from compas.utilities import pairwise + + +def test_polygon(): + points = [[0, 0, x] for x in range(5)] + polygon = Polygon(points) + assert polygon.points == points + assert polygon.lines == [(a, b) for a, b in pairwise(points + points[:1])] + + +def test_equality(): + points1 = [[0, 0, x] for x in range(5)] + polygon1 = Polygon(points1) + points2 = [[0, 0, x] for x in range(6)] + polygon2 = Polygon(points2) + points3 = [[0, 0, x] for x in range(5)] + [[0, 0, 0]] + polygon3 = Polygon(points3) + assert polygon1 == polygon1 + assert polygon1 == points1 + assert points1 == polygon1 + assert polygon1 != polygon2 + assert polygon2 != polygon1 + assert polygon1 != points2 + assert points2 != polygon1 + assert polygon1 != 1 + assert polygon1 == polygon3 + + +def test___repr__(): + points = [[0, 0, x] for x in range(5)] + polygon = Polygon(points) + assert polygon == eval(repr(polygon)) + + +def test___getitem__(): + points = [[0, 0, x] for x in range(5)] + polygon = Polygon(points) + for x in range(5): + assert polygon[x] == [0, 0, x] + with pytest.raises(IndexError): + polygon[6] = [0, 0, 6] + + +def test___setitem__(): + points = [[0, 0, x] for x in range(5)] + polygon = Polygon(points) + point = [1, 1, 4] + polygon[4] = point + assert polygon[4] == point + assert isinstance(polygon[4], Point) + assert polygon.lines[-2].end == point \ No newline at end of file diff --git a/tests/compas/geometry/test_polyline.py b/tests/compas/geometry/test_polyline.py index 6ba2410ac5e5..0b9dd306f00f 100644 --- a/tests/compas/geometry/test_polyline.py +++ b/tests/compas/geometry/test_polyline.py @@ -1,7 +1,56 @@ -import compas -import pytest -from compas.geometry import Polyline, Point import math +import pytest + +from compas.geometry import Point +from compas.geometry import Polyline +from compas.utilities import pairwise + + +def test_polyline(): + points = [[0, 0, x] for x in range(5)] + polyline = Polyline(points) + assert polyline.points == points + assert polyline.lines == [(a, b) for a, b in pairwise(points)] + + +def test_equality(): + points1 = [[0, 0, x] for x in range(5)] + polyline1 = Polyline(points1) + points2 = [[0, 0, x] for x in range(6)] + polyline2 = Polyline(points2) + assert polyline1 == polyline1 + assert polyline1 == points1 + assert points1 == polyline1 + assert polyline1 != polyline2 + assert polyline2 != polyline1 + assert polyline1 != points2 + assert points2 != polyline1 + assert polyline1 != 1 + + +def test___repr__(): + points = [[0, 0, x] for x in range(5)] + polyline = Polyline(points) + assert polyline == eval(repr(polyline)) + + +def test___getitem__(): + points = [[0, 0, x] for x in range(5)] + polyline = Polyline(points) + for x in range(5): + assert polyline[x] == [0, 0, x] + with pytest.raises(IndexError): + polyline[6] = [0, 0, 6] + + +def test___setitem__(): + points = [[0, 0, x] for x in range(5)] + polyline = Polyline(points) + point = [1, 1, 4] + polyline[4] = point + assert polyline[4] == point + assert isinstance(polyline[4], Point) + assert polyline.lines[-1].end == point @pytest.mark.parametrize('coords,expected', [ From b3db0eb2680203630a4e2e4c255072f41b403b44 Mon Sep 17 00:00:00 2001 From: beverly Date: Mon, 15 Mar 2021 09:37:56 +0100 Subject: [PATCH 2/4] use allclose --- src/compas/geometry/primitives/polygon.py | 7 ++++--- src/compas/geometry/primitives/polyline.py | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/compas/geometry/primitives/polygon.py b/src/compas/geometry/primitives/polygon.py index 15752418d45f..b847478360db 100644 --- a/src/compas/geometry/primitives/polygon.py +++ b/src/compas/geometry/primitives/polygon.py @@ -4,17 +4,18 @@ import math +from compas.geometry import allclose +from compas.geometry import area_polygon from compas.geometry import cross_vectors from compas.geometry import centroid_polygon -from compas.geometry import area_polygon from compas.geometry import is_coplanar from compas.geometry import is_polygon_convex from compas.geometry import transform_points +from compas.geometry.primitives import Line from compas.geometry.primitives import Primitive from compas.geometry.primitives import Point from compas.geometry.primitives import Vector -from compas.geometry.primitives import Line from compas.utilities import pairwise @@ -165,7 +166,7 @@ def __iter__(self): def __eq__(self, other): if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): return False - return all(a == b for a, b in zip(self, other)) + return allclose(self, other) # ========================================================================== # constructors diff --git a/src/compas/geometry/primitives/polyline.py b/src/compas/geometry/primitives/polyline.py index c1cf4c2c3ffd..5c7c5daa990f 100644 --- a/src/compas/geometry/primitives/polyline.py +++ b/src/compas/geometry/primitives/polyline.py @@ -2,12 +2,13 @@ from __future__ import absolute_import from __future__ import division +from compas.geometry import allclose from compas.geometry import transform_points +from compas.geometry.predicates import is_point_on_line +from compas.geometry.primitives import Line from compas.geometry.primitives import Primitive from compas.geometry.primitives import Point -from compas.geometry.primitives import Line -from compas.geometry.predicates import is_point_on_line from compas.utilities import pairwise @@ -127,7 +128,7 @@ def __iter__(self): def __eq__(self, other): if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): return False - return all(a == b for a, b in zip(self, other)) + return allclose(self, other) # ========================================================================== # constructors From f238702b36387e0930f580f1b270b23d2d793a06 Mon Sep 17 00:00:00 2001 From: beverly Date: Mon, 15 Mar 2021 09:46:23 +0100 Subject: [PATCH 3/4] even more allclose --- src/compas/geometry/primitives/frame.py | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/compas/geometry/primitives/frame.py b/src/compas/geometry/primitives/frame.py index 8098fa51fc27..4b83aef1b9f1 100644 --- a/src/compas/geometry/primitives/frame.py +++ b/src/compas/geometry/primitives/frame.py @@ -1,25 +1,24 @@ from __future__ import print_function -import math - -from compas.geometry import cross_vectors -from compas.geometry import subtract_vectors -from compas.geometry import matrix_from_basis_vectors -from compas.geometry import basis_vectors_from_matrix -from compas.geometry import quaternion_from_matrix -from compas.geometry import matrix_from_quaternion +from compas.geometry import allclose +from compas.geometry import argmax from compas.geometry import axis_angle_vector_from_matrix -from compas.geometry import matrix_from_axis_angle_vector +from compas.geometry import basis_vectors_from_matrix +from compas.geometry import cross_vectors +from compas.geometry import decompose_matrix from compas.geometry import euler_angles_from_matrix +from compas.geometry import matrix_from_axis_angle_vector +from compas.geometry import matrix_from_basis_vectors from compas.geometry import matrix_from_euler_angles -from compas.geometry import decompose_matrix +from compas.geometry import matrix_from_quaternion +from compas.geometry import quaternion_from_matrix +from compas.geometry import subtract_vectors from compas.geometry import Transformation -from compas.geometry import argmax -from compas.geometry.primitives import Primitive from compas.geometry.primitives import Point -from compas.geometry.primitives import Vector +from compas.geometry.primitives import Primitive from compas.geometry.primitives import Quaternion +from compas.geometry.primitives import Vector __all__ = ['Frame'] @@ -182,11 +181,7 @@ def __iter__(self): def __eq__(self, other, tol=1e-05): if not hasattr(other, '__iter__') or not hasattr(other, '__len__') or len(self) != len(other): return False - for v1, v2 in zip(self, other): - for a, b in zip(v1, v2): - if math.fabs(a - b) > tol: - return False - return True + return allclose(self, other) # ========================================================================== # constructors @@ -744,5 +739,4 @@ def transform(self, T): if __name__ == '__main__': import doctest - from compas.geometry import allclose # noqa: F401 doctest.testmod(globs=globals()) From 364b32be1e4e601384814edb2cb7e352f249b2cd Mon Sep 17 00:00:00 2001 From: beverly Date: Mon, 15 Mar 2021 09:57:18 +0100 Subject: [PATCH 4/4] imports --- src/compas/geometry/primitives/polygon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compas/geometry/primitives/polygon.py b/src/compas/geometry/primitives/polygon.py index b847478360db..7f43cfc9d30d 100644 --- a/src/compas/geometry/primitives/polygon.py +++ b/src/compas/geometry/primitives/polygon.py @@ -13,8 +13,8 @@ from compas.geometry import transform_points from compas.geometry.primitives import Line -from compas.geometry.primitives import Primitive from compas.geometry.primitives import Point +from compas.geometry.primitives import Primitive from compas.geometry.primitives import Vector from compas.utilities import pairwise