Feat: Add `check_coplanar` kwarg to `Line.intersect_line` #320

volkoshkursk · 2022-11-10T12:42:28Z

Hello! This is my fix of issue intersection line are not complanar with line in plane #319. I have added parameter tol to Plane.intersect_line, which is passed to Line.is_coplanar as keyword argument. Also I have added appropriate parameter to Triangle.orthocenter and Line.Segment.intersect_line_segment.

src/skspatial/objects/line.py

ajhynes7 · 2022-11-20T20:05:47Z

src/skspatial/objects/line.py

@@ -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):


Suggested change

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

if check_coplanar and not self.is_coplanar(other):

This still needs to be updated.

src/skspatial/objects/line_segment.py

ajhynes7 · 2022-11-20T20:08:32Z

src/skspatial/objects/line_segment.py

@@ -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:


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:

ajhynes7 · 2022-11-20T20:09:42Z

src/skspatial/objects/triangle.py

@@ -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:


Suggested change

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

def orthocenter(self, **kwargs) -> Point:

src/skspatial/objects/triangle.py

Co-authored-by: Andrew Hynes <andrewjhynes@gmail.com>

ajhynes7

It'd be also good if you added a unit test which calls the intersect_line method with is_coplanar=False.

ajhynes7 · 2022-11-23T00:37:26Z

src/skspatial/objects/line.py

@@ -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


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.

ajhynes7 · 2022-11-23T00:39:16Z

src/skspatial/objects/line.py

@@ -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):


This still needs to be updated.

ajhynes7 · 2022-11-23T00:41:40Z

src/skspatial/objects/line_segment.py

        """
        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.
+


This needs to change to the following:

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

ajhynes7 · 2022-11-23T00:43:59Z

src/skspatial/objects/triangle.py

+        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.


This needs to change to the following:

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

ajhynes7

You still need to update the docstring of Line.intersect_line.

ajhynes7 · 2022-11-27T21:37:43Z

src/skspatial/__init__.py

@@ -9,4 +9,4 @@
 except ModuleNotFoundError:
    import importlib_metadata  # type: ignore

-__version__ = importlib_metadata.version("scikit-spatial")
+# __version__ = importlib_metadata.version("scikit-spatial")


This shouldn't be commented out.

I forget to remove this comment. Now it is fixed.

ajhynes7 · 2022-12-03T14:25:25Z

Hi, there are some linting errors that need to be fixed.
https://github.com/ajhynes7/scikit-spatial/actions/runs/3603902670/jobs/6072652436

ajhynes7 · 2022-12-08T20:21:34Z

Hi, this PR is still failing.

https://github.com/ajhynes7/scikit-spatial/actions/runs/3627753010/jobs/6131842217

I suggest installing pre-commit and running this locally:

pre-commit run --all-files

Then commit the changes.

ajhynes7 · 2022-12-29T16:49:06Z

@volkoshkursk are you still interested in merging this?

for more information, see https://pre-commit.ci

sonarcloud · 2022-12-29T19:39:09Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

codecov · 2022-12-29T19:48:32Z

Codecov Report

Merging #320 (937aff2) into master (b62d39e) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 937aff2 differs from pull request most recent head c253738. Consider uploading reports for the commit c253738 to get more accurate results

@@           Coverage Diff           @@
##           master     #320   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files          21       21           
  Lines         948      948           
=======================================
  Hits          946      946           
  Misses          2        2

Impacted Files	Coverage Δ
src/skspatial/objects/line.py	`100.00% <100.00%> (ø)`
src/skspatial/objects/line_segment.py	`100.00% <100.00%> (ø)`
src/skspatial/objects/triangle.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ajhynes7 · 2022-12-29T19:50:29Z

I set up pre-commit.ci to make the fixes automatically.

volkoshkursk · 2022-12-30T08:11:37Z

Sorry, I had got sick and was not able to fix this. Thank you very much and Happy New Year!

ajhynes7 · 2022-12-30T21:19:31Z

You're welcome, happy new year to you too!

Feat: Add parameter tol to intersect_line

657cb6c

ajhynes7 requested changes Nov 20, 2022

View reviewed changes

volkoshkursk and others added 3 commits November 21, 2022 19:01

Update src/skspatial/objects/line.py

c376ac0

Co-authored-by: Andrew Hynes <andrewjhynes@gmail.com>

Update src/skspatial/objects/line_segment.py

67007d8

Co-authored-by: Andrew Hynes <andrewjhynes@gmail.com>

Update src/skspatial/objects/triangle.py

1b2789c

Co-authored-by: Andrew Hynes <andrewjhynes@gmail.com>

ajhynes7 requested changes Nov 23, 2022

View reviewed changes

ajhynes7 changed the title ~~Feat: Add parameter tol to intersect_line~~ Feat: Add check_coplanar kwarg to Line.intersect_line Nov 23, 2022

Fix: rewrite docs, functions && add tests

1e6f7dc

ajhynes7 requested changes Nov 27, 2022

View reviewed changes

volkoshkursk and others added 3 commits November 28, 2022 13:03

fix: remove test comment.

869efc0

docs: update docs of line.intersect_line.

4f4a101

Merge branch 'master' into master

6d043ab

docs: fix docs with respect to E501.

fd785bb

ajhynes7 force-pushed the master branch 2 times, most recently from 702e0be to 31ee17e Compare December 29, 2022 02:45

ajhynes7 and others added 2 commits December 29, 2022 16:06

Merge branch 'master' into master

937aff2

[pre-commit.ci] auto fixes from pre-commit.com hooks

c253738

for more information, see https://pre-commit.ci

ajhynes7 merged commit eaa889d into ajhynes7:master Dec 29, 2022

ajhynes7 linked an issue Dec 29, 2022 that may be closed by this pull request

intersection line are not complanar with line in plane #319

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Add `check_coplanar` kwarg to `Line.intersect_line` #320

Feat: Add `check_coplanar` kwarg to `Line.intersect_line` #320

volkoshkursk commented Nov 10, 2022

ajhynes7 Nov 20, 2022

ajhynes7 Nov 23, 2022

ajhynes7 Nov 20, 2022

ajhynes7 Nov 20, 2022

ajhynes7 left a comment

ajhynes7 Nov 23, 2022

ajhynes7 Nov 23, 2022

ajhynes7 Nov 23, 2022

ajhynes7 Nov 23, 2022

ajhynes7 left a comment

ajhynes7 Nov 27, 2022

volkoshkursk Nov 28, 2022

ajhynes7 commented Dec 3, 2022

ajhynes7 commented Dec 8, 2022

ajhynes7 commented Dec 29, 2022

sonarcloud bot commented Dec 29, 2022

codecov bot commented Dec 29, 2022

ajhynes7 commented Dec 29, 2022

volkoshkursk commented Dec 30, 2022 •

edited

Loading

ajhynes7 commented Dec 30, 2022

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

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

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

Feat: Add check_coplanar kwarg to Line.intersect_line #320

Feat: Add check_coplanar kwarg to Line.intersect_line #320

Conversation

volkoshkursk commented Nov 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajhynes7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajhynes7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajhynes7 commented Dec 3, 2022

ajhynes7 commented Dec 8, 2022

ajhynes7 commented Dec 29, 2022

sonarcloud bot commented Dec 29, 2022

codecov bot commented Dec 29, 2022

Codecov Report

ajhynes7 commented Dec 29, 2022

volkoshkursk commented Dec 30, 2022 • edited Loading

ajhynes7 commented Dec 30, 2022

Feat: Add `check_coplanar` kwarg to `Line.intersect_line` #320

Feat: Add `check_coplanar` kwarg to `Line.intersect_line` #320

volkoshkursk commented Dec 30, 2022 •

edited

Loading