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

Feat: Add check_coplanar kwarg to Line.intersect_line #320

Merged
merged 11 commits into from
Dec 29, 2022
6 changes: 4 additions & 2 deletions src/skspatial/objects/line.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ def distance_line(self, other: Line) -> np.float64:

return distance

def intersect_line(self, other: Line, **kwargs) -> Point:
def intersect_line(self, other: Line, tol: np.float64 = None, **kwargs) -> Point:
volkoshkursk marked this conversation as resolved.
Show resolved Hide resolved
"""
Intersect the line with another.

Expand All @@ -485,6 +485,8 @@ def intersect_line(self, other: Line, **kwargs) -> Point:
----------
other : Line
Other line.
tol : np.float64, optional
Threshold below which values are considered zero. This value is passed to is_coplanar
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be changed to the following:

check_coplanar : bool
    Check that the lines are coplanar (default True). If False, this method may not return an actual intersection point, but an approximate one.

kwargs : dict, optional
Additional keywords passed to :meth:`Vector.is_parallel`.

Expand Down Expand Up @@ -560,7 +562,7 @@ def intersect_line(self, other: Line, **kwargs) -> Point:
if self.direction.is_parallel(other.direction, **kwargs):
raise ValueError("The lines must not be parallel.")

if not self.is_coplanar(other):
if not self.is_coplanar(other, tol=tol):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not self.is_coplanar(other, tol=tol):
if check_coplanar and not self.is_coplanar(other):

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be updated.

raise ValueError("The lines must be coplanar.")

# Vector from line A to line B.
Expand Down
7 changes: 5 additions & 2 deletions src/skspatial/objects/line_segment.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,17 @@ def contains_point(self, point: array_like, **kwargs) -> bool:

return math.isclose(similarity, -1, **kwargs)

def intersect_line_segment(self, other: LineSegment) -> Point:
def intersect_line_segment(self, other: LineSegment, tol: np.float64 = None) -> Point:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method can just pass all kwargs along to the intersect_line method.

Suggested change
def intersect_line_segment(self, other: LineSegment, tol: np.float64 = None) -> Point:
def intersect_line_segment(self, other: LineSegment, **kwargs) -> Point:

"""
Intersect the line segment with another.

Parameters
----------
other : LineSegment

tol : np.float64, optional
Threshold below which values are considered zero. This value is passed to is_coplanar as tol and to is_collinear as abs_tol.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to change to the following:

kwargs : dict, optional
    Additional keyword arguments passed to :meth:`Line.intersect_line`.

Returns
-------
Point
Expand Down Expand Up @@ -145,7 +148,7 @@ def intersect_line_segment(self, other: LineSegment) -> Point:
line_a = Line.from_points(self.point_a, self.point_b)
line_b = Line.from_points(other.point_a, other.point_b)

point_intersection = line_a.intersect_line(line_b)
point_intersection = line_a.intersect_line(line_b, tol=tol, abs_tol=tol) if tol is not None else line_a.intersect_line(line_b)
volkoshkursk marked this conversation as resolved.
Show resolved Hide resolved

point_on_segment_a = self.contains_point(point_intersection)
point_on_segment_b = other.contains_point(point_intersection)
Expand Down
10 changes: 8 additions & 2 deletions src/skspatial/objects/triangle.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,12 +498,18 @@ def altitude(self, vertex: str) -> Line:

raise ValueError("The vertex must be 'A', 'B', or 'C'.")

def orthocenter(self) -> Point:
def orthocenter(self, tol: np.float64 = None) -> Point:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def orthocenter(self, tol: np.float64 = None) -> Point:
def orthocenter(self, **kwargs) -> Point:

"""
Return the orthocenter of the triangle.

The orthocenter is the intersection point of the three altitudes.

Parameters
----------
tol : np.float64, optional
Threshold below which values are considered zero. This value is passed to is_coplanar as tol and to is_collinear as abs_tol.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to change to the following:

kwargs : dict, optional
    Additional keywords passed to :meth:`Line.intersect_line`.



Returns
-------
Point
Expand All @@ -523,7 +529,7 @@ def orthocenter(self) -> Point:
line_alt_a = self.altitude('A')
line_alt_b = self.altitude('B')

return line_alt_a.intersect_line(line_alt_b)
return line_alt_a.intersect_line(line_alt_b, tol=tol, abs_tol=tol) if tol is not None else line_alt_a.intersect_line(line_alt_b)
volkoshkursk marked this conversation as resolved.
Show resolved Hide resolved

def classify(self, **kwargs: float) -> str:
"""
Expand Down