Skip to content

Commit

Permalink
fix(polygon): Ensure intersected vertices are sorted before insertion
Browse files Browse the repository at this point in the history
This commit fixes a bug that Abraham found here:
https://discourse.ladybug.tools/t/df-adjacency-issue/10784

I am also taking the opportunity to fix a few PEP8 issues in the code (notably a lot of blank lines with white spaces).
  • Loading branch information
chriswmackey authored and Chris Mackey committed Aug 27, 2020
1 parent 1625873 commit 94c9b0a
Show file tree
Hide file tree
Showing 19 changed files with 77 additions and 48 deletions.
2 changes: 1 addition & 1 deletion ladybug_geometry/_mesh.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def __hash__(self):

def __eq__(self, other):
return isinstance(other, MeshBase) and self.__key() == other.__key()

def __ne__(self, other):
return not self.__eq__(other)

Expand Down
10 changes: 5 additions & 5 deletions ladybug_geometry/_polyline.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@

def _group_vertices(segments, tolerance):
"""Get lists of joined polyline vertices from segments.
Args:
segments: An array of LineSegment objects.
tolerance: The minimum difference in X, Y, and Z values at which Points
are considred equivalent. Segments with points that match within the
tolerance will be joined.
Returns:
A list of lists vertices that represent joined polylines.
"""
Expand All @@ -29,14 +29,14 @@ def _group_vertices(segments, tolerance):

def _build_polyline(base_seg, other_segs, tol):
"""Attempt to build a list of polyline vertices from a base segment.
Args:
base_seg: A LineSegment to serve as the base of the Polyline.
other_segs: A list of other LineSegment objects to attempt to
connect to the base_seg. This method will delete any segments
that are successfully connected to the output from this list.
that are successfully connected to the output from this list.
tol: The tolerance to be used for connecting the line.
Returns:
A list of vertices that represent the longest Polyline to which the
base_seg can be a part of given the other_segs as connections.
Expand Down
2 changes: 1 addition & 1 deletion ladybug_geometry/geometry2d/_1d.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def __hash__(self):

def __eq__(self, other):
return isinstance(other, Base1DIn2D) and self.__key() == other.__key()

def __ne__(self, other):
return not self.__eq__(other)

Expand Down
6 changes: 3 additions & 3 deletions ladybug_geometry/geometry2d/arc.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Arc2D(object):
"""
__slots__ = ('_c', '_r', '_a1', '_a2', '_cos_a1', '_sin_a1', '_cos_a2', '_sin_a2')

def __init__(self, c, r, a1=0, a2=2*math.pi):
def __init__(self, c, r, a1=0, a2=2 * math.pi):
"""Initialize Arc2D."""
assert isinstance(c, Point2D), "Expected Point2D. Got {}.".format(type(c))
assert r > 0, 'Arc radius must be greater than 0. Got {}.'.format(r)
Expand Down Expand Up @@ -246,7 +246,7 @@ def subdivide_evenly(self, number):
while parameter <= 1:
sub_pts.append(self.point_at(parameter))
parameter += interval
return sub_pts
return sub_pts

def point_at(self, parameter):
"""Get a point at a given fraction along the arc.
Expand Down Expand Up @@ -369,7 +369,7 @@ def split_line_infinite(self, line_ray):

def to_polyline(self, divisions, interpolated=True):
"""Get this Arc2D as an approximated Polyline2D.
Args:
divisions: The number of segments into which the arc will be divided.
interpolated: Boolean to note whether the polyline should be interpolated
Expand Down
2 changes: 1 addition & 1 deletion ladybug_geometry/geometry2d/line.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,4 @@ def __eq__(self, other):

def __repr__(self):
return 'LineSegment2D (<%.2f, %.2f> to <%.2f, %.2f>)' % \
(self.p.x, self.p.y, self.p.x + self.v.x, self.p.y + self.v.y)
(self.p.x, self.p.y, self.p.x + self.v.x, self.p.y + self.v.y)
1 change: 0 additions & 1 deletion ladybug_geometry/geometry2d/pointvector.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,4 +426,3 @@ def __gt__(self, other):
"""
if isinstance(other, Vector2D):
return self.x > other.x

5 changes: 3 additions & 2 deletions ladybug_geometry/geometry2d/polygon.py
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,8 @@ def _insert_updates_in_order(polygon, polygon_updates):
first is the index of the segment to be updated and the second is
the point to insert.
"""
poly_points = list(polygon.vertices)
polygon_updates.sort(key=lambda x: x[0]) # sort updates by order of insertion
poly_points = list(polygon.vertices) # convert tuple to mutable list
last_i = -1
colinear_count = 0
for update in polygon_updates[::-1]: # traverse backwards to preserve order
Expand Down Expand Up @@ -919,4 +920,4 @@ def __eq__(self, other):
return isinstance(other, Polygon2D) and self.__key() == other.__key()

def __repr__(self):
return 'Polygon2D ({} vertices)'.format(len(self))
return 'Polygon2D ({} vertices)'.format(len(self))
2 changes: 1 addition & 1 deletion ladybug_geometry/geometry2d/polyline.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ def to_array(self):

def to_polygon(self, tolerance):
"""Get a Polygon2D derived from this object.
If the polyline is closed to within the tolerance, the segments of this
polyline and the resulting polygon will match. Otherwise, an extra
LineSegment2D will be added to connect the start and end of the polyline.
Expand Down
4 changes: 2 additions & 2 deletions ladybug_geometry/geometry3d/_1d.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def to_dict(self):

def __copy__(self):
return self.__class__(self.p, self.v)

def __key(self):
"""A tuple based on the object properties, useful for hashing."""
return (hash(self.p), hash(self.v))
Expand All @@ -146,7 +146,7 @@ def __hash__(self):

def __eq__(self, other):
return isinstance(other, Base1DIn3D) and self.__key() == other.__key()

def __ne__(self, other):
return not self.__eq__(other)

Expand Down
2 changes: 1 addition & 1 deletion ladybug_geometry/geometry3d/_2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __hash__(self):

def __eq__(self, other):
return isinstance(other, Base2DIn3D) and self.__key() == other.__key()

def __ne__(self, other):
return not self.__eq__(other)

Expand Down
4 changes: 2 additions & 2 deletions ladybug_geometry/geometry3d/arc.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Arc3D(object):
"""
__slots__ = ('_plane', '_arc2d')

def __init__(self, plane, radius, a1=0, a2=2*math.pi):
def __init__(self, plane, radius, a1=0, a2=2 * math.pi):
"""Initialize Arc3D."""
assert isinstance(plane, Plane), "Expected Plane. Got {}.".format(type(plane))
self._plane = plane
Expand Down Expand Up @@ -326,7 +326,7 @@ def split_with_plane(self, plane):

def to_polyline(self, divisions, interpolated=True):
"""Get this Arc3D as an approximated Polyline3D.
Args:
divisions: The number of segments into which the arc will be divided.
interpolated: Boolean to note whether the polyline should be interpolated
Expand Down
4 changes: 2 additions & 2 deletions ladybug_geometry/geometry3d/line.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"""3D Line Segment"""
from __future__ import division

from .pointvector import Point3D, Vector3D
from .pointvector import Point3D
from ._1d import Base1DIn3D


Expand Down Expand Up @@ -243,7 +243,7 @@ def _u_in(self, u):

def __abs__(self):
return abs(self.v)

def __copy__(self):
return LineSegment3D(self.p, self.v)

Expand Down
4 changes: 2 additions & 2 deletions ladybug_geometry/geometry3d/plane.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,10 @@ def xy_to_xyz(self, point):
_v = (self.y.x * point.y, self.y.y * point.y, self.y.z * point.y)
return Point3D(
self.o.x + _u[0] + _v[0], self.o.y + _u[1] + _v[1], self.o.z + _u[2] + _v[2])

def is_point_above(self, point):
"""Test if a given point is above or below this plane.
Above is defined as being on the side of the plane that the plane normal
is pointing towards.
Expand Down
2 changes: 1 addition & 1 deletion ladybug_geometry/geometry3d/polyface.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ def __copy__(self):
_new_poly = Polyface3D(self.vertices, self.face_indices, self.edge_information)
_new_poly._faces = self._faces
return _new_poly

def __key(self):
"""A tuple based on the object properties, useful for hashing."""
return tuple(hash(pt) for pt in self._vertices) + \
Expand Down
2 changes: 1 addition & 1 deletion ladybug_geometry/geometry3d/polyline.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def from_polyline2d(cls, polyline2d, plane=Plane()):
plane: A Plane in which the Polyline2D sits.
"""
return Polyline3D((plane.xy_to_xyz(pt) for pt in polyline2d.vertices),
polyline2d.interpolated)
polyline2d.interpolated)

@property
def segments(self):
Expand Down
4 changes: 2 additions & 2 deletions ladybug_geometry/geometry3d/ray.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Ray3D(Base1DIn3D):
def __init__(self, p, v):
"""Initialize Ray3D."""
Base1DIn3D.__init__(self, p, v)

@classmethod
def from_array(cls, ray_array):
""" Create a Ray3D from a nested array with a point and a vector.
Expand Down Expand Up @@ -107,7 +107,7 @@ def to_array(self):

def _u_in(self, u):
return u >= 0.0

def __copy__(self):
return Ray3D(self.p, self.v)

Expand Down
1 change: 1 addition & 0 deletions ladybug_geometry/intersection2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ def closest_point2d_on_line2d(point, line_ray):
u = max(min(u, 1.0), 0.0)
return Point2D(line_ray.p.x + u * line_ray.v.x, line_ray.p.y + u * line_ray.v.y)


def closest_point2d_between_line2d(line_ray_a, line_ray_b):
"""Get the two closest Point2D between two LineSegment2D objects.
Expand Down
10 changes: 5 additions & 5 deletions tests/mesh3d_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@


def test_mesh3d_init():
"""Test the initalization of Mesh3D objects and basic properties."""
"""Test the initialization of Mesh3D objects and basic properties."""
pts = (Point3D(0, 0, 2), Point3D(0, 2, 2), Point3D(2, 2, 2), Point3D(2, 0, 2))
mesh = Mesh3D(pts, [(0, 1, 2, 3)])
str(mesh) # test the string representation of the object
Expand Down Expand Up @@ -78,7 +78,7 @@ def test_face_normals():


def test_mesh3d_incorrect():
"""Test the initalization of Mesh3D objects with incorrect values."""
"""Test the initialization of Mesh3D objects with incorrect values."""
pts = (Point3D(0, 0), Point3D(0, 2), Point3D(2, 2), Point3D(2, 0), Point3D(4, 0))
with pytest.raises(AssertionError):
Mesh3D(pts, [(0, 1, 2, 3, 5)]) # too many vertices in a face
Expand All @@ -93,7 +93,7 @@ def test_mesh3d_incorrect():


def test_mesh3d_init_two_faces():
"""Test the initalization of Mesh3D objects with two faces."""
"""Test the initialization of Mesh3D objects with two faces."""
pts = (Point3D(0, 0, 2), Point3D(0, 2, 2), Point3D(2, 2, 2),
Point3D(2, 0, 2), Point3D(4, 0, 2))
mesh = Mesh3D(pts, [(0, 1, 2, 3), (2, 3, 4)])
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_mesh3d_init_two_faces():


def test_mesh3d_init_from_face_vertices():
"""Test the initalization of Mesh3D from_face_vertices."""
"""Test the initialization of Mesh3D from_face_vertices."""
face_1 = (Point3D(0, 0, 2), Point3D(0, 2, 2), Point3D(2, 2, 2), Point3D(2, 0, 2))
face_2 = (Point3D(2, 2, 2), Point3D(2, 0, 2), Point3D(4, 0, 2))
mesh_1 = Mesh3D.from_face_vertices([face_1, face_2])
Expand Down Expand Up @@ -153,7 +153,7 @@ def test_mesh3d_init_from_face_vertices():


def test_mesh3d_from_mesh2d():
"""Test the initalization of Mesh3D objects from_mesh2d."""
"""Test the initialization of Mesh3D objects from_mesh2d."""
pts = (Point2D(0, 0), Point2D(0, 2), Point2D(2, 2), Point2D(2, 0))
mesh_2d = Mesh2D(pts, [(0, 1, 2, 3)])
plane = Plane(Vector3D(1, 0, 0), Point3D(0, 0, 0))
Expand Down
Loading

0 comments on commit 94c9b0a

Please sign in to comment.