Fix percentage distance methods (#339)

* Corrected calculation of arc length * Corrected make percentage method use percentage not fraction * Deprecated get_coordinate_from_percentage_distance methods. Replaced by update to get_coordinate_from_distance methods. * update tests * Added deprecation warnings * Added more tests for updated get_coordinate_from_distance method (new tests for new optional arguments) * Added more tests for updated get_coordinate_from_distance method (tests for warnings when multiple arguments are given) * Added more tests for updated get_coordinate_from_distance method (test for arc drawn with negative radius) * Added test for arc with negative radius for deprecated get_coordinate_from_percentage_distance arc method * Added tests to check error is raised when no distance, percentage or fraction is provided * Update src/ansys/motorcad/core/geometry.py Co-authored-by: James Packer <[email protected]> * Update src/ansys/motorcad/core/geometry.py Co-authored-by: James Packer <[email protected]> * Update src/ansys/motorcad/core/geometry.py Co-authored-by: James Packer <[email protected]> * Update src/ansys/motorcad/core/geometry.py Co-authored-by: James Packer <[email protected]> * Update src/ansys/motorcad/core/geometry.py Co-authored-by: James Packer <[email protected]> --------- Co-authored-by: Jack Davies <[email protected]> Co-authored-by: James Packer <[email protected]>
ansys · Nov 27, 2024 · 3061cb5 · 3061cb5
1 parent eab6e22
commit 3061cb5
Show file tree

Hide file tree

Showing 2 changed files with 199 additions and 37 deletions.
diff --git a/src/ansys/motorcad/core/geometry.py b/src/ansys/motorcad/core/geometry.py
@@ -25,6 +25,7 @@
 from copy import deepcopy
 from enum import Enum
 from math import atan2, cos, degrees, inf, isclose, radians, sin, sqrt
+from warnings import warn
 
 GEOM_TOLERANCE = 1e-6
 
@@ -1195,51 +1196,74 @@ def mirror(self, mirror_line):
         else:
             raise Exception("Line can only be mirrored about Line()")
 
-    def get_coordinate_from_percentage_distance(self, ref_coordinate, percentage):
-        """Get the coordinate at the percentage distance along the line from the reference.
+    def get_coordinate_from_percentage_distance(self, ref_coordinate, fraction):
+        """Get the coordinate at a fractional distance along the line from the reference coord.
+
+        .. note::
+           This method is deprecated. Use the :func:`Line.get_coordinate_from_distance`
+           method with the `fraction = ` or `percentage =` argument.
 
         Parameters
         ----------
         ref_coordinate : Coordinate
             Entity reference coordinate.
 
-        percentage : float
-            Percentage distance along Line.
+        fraction : float
+            Fractional distance along Line.
 
         Returns
         -------
         Coordinate
-            Coordinate at percentage distance along Line.
+            Coordinate at fractional distance along Line.
         """
-        if ref_coordinate == self.end:
-            coordinate_1 = self.end
-            coordinate_2 = self.start
-        else:
-            coordinate_1 = self.start
-            coordinate_2 = self.end
-
-        t = (self.length * percentage) / self.length
-        x = ((1 - t) * coordinate_1.x) + (t * coordinate_2.x)
-        y = ((1 - t) * coordinate_1.y) + (t * coordinate_2.y)
-
-        return Coordinate(x, y)
+        warn(
+            "get_coordinate_from_percentage_distance() WILL BE DEPRECATED SOON - "
+            "USE get_coordinate_from_distance instead with the `fraction = ` or `percentage = ` "
+            "optional argument",
+            DeprecationWarning,
+        )
+        return self.get_coordinate_from_distance(ref_coordinate, fraction=fraction)
 
-    def get_coordinate_from_distance(self, ref_coordinate, distance):
+    def get_coordinate_from_distance(
+        self, ref_coordinate, distance=None, fraction=None, percentage=None
+    ):
         """Get the coordinate at the specified distance along the line from the reference.
 
         Parameters
         ----------
         ref_coordinate : Coordinate
             Entity reference coordinate.
 
-        distance : float
+        distance : float, optional
             Distance along Line.
 
+        fraction : float, optional
+            Fractional distance along Line.
+
+        percentage : float, optional
+            Percentage distance along Line.
+
         Returns
         -------
         Coordinate
             Coordinate at distance along Line.
         """
+        if not distance and not fraction and not percentage:
+            raise Exception("You must provide either a distance, fraction or percentage.")
+
+        if distance and fraction:
+            warn("Both distance and fraction provided. Using distance.", UserWarning)
+        if distance and percentage:
+            warn("Both distance and percentage provided. Using distance.", UserWarning)
+
+        if not distance:
+            if fraction and percentage:
+                warn("Both fraction and percentage provided. Using fraction.", UserWarning)
+            if fraction:
+                distance = self.length * fraction
+            elif percentage:
+                distance = self.length * (percentage / 100)
+
         if ref_coordinate == self.end:
             coordinate_1 = self.end
             coordinate_2 = self.start
@@ -1374,42 +1398,74 @@ def midpoint(self):
         x_shift, y_shift = rt_to_xy(abs(self.radius), angle)
         return Coordinate(self.centre.x + x_shift, self.centre.y + y_shift)
 
-    def get_coordinate_from_percentage_distance(self, ref_coordinate, percentage):
-        """Get the coordinate at the percentage distance along the arc from the reference coord.
+    def get_coordinate_from_percentage_distance(self, ref_coordinate, fraction):
+        """Get the coordinate at a fractional distance along the arc from the reference coord.
+
+        .. note::
+           This method is deprecated. Use the :func:`Arc.get_coordinate_from_distance`
+           method with the `fraction = ` or `percentage =` argument.
 
         Parameters
         ----------
         ref_coordinate : Coordinate
             Entity reference coordinate.
 
-        percentage : float
-            Percentage distance along Arc.
+        fraction : float
+            Fractional distance along Arc.
 
         Returns
         -------
         Coordinate
-            Coordinate at percentage distance along Arc.
+            Coordinate at fractional distance along Arc.
         """
-        length = self.length * percentage
-
-        return self.get_coordinate_from_distance(ref_coordinate, length)
+        warn(
+            "get_coordinate_from_percentage_distance() WILL BE DEPRECATED SOON - "
+            "USE get_coordinate_from_distance instead with the `fraction = ` or `percentage = ` "
+            "optional argument",
+            DeprecationWarning,
+        )
+        return self.get_coordinate_from_distance(ref_coordinate, fraction=fraction)
 
-    def get_coordinate_from_distance(self, ref_coordinate, distance):
+    def get_coordinate_from_distance(
+        self, ref_coordinate, distance=None, fraction=None, percentage=None
+    ):
         """Get the coordinate at the specified distance along the arc from the reference coordinate.
 
         Parameters
         ----------
         ref_coordinate : Coordinate
            Entity reference coordinate.
 
-        distance : float
+        distance : float, optional
             Distance along arc.
 
+        fraction : float, optional
+            Fractional distance along Arc.
+
+        percentage : float, optional
+            Percentage distance along Arc.
+
         Returns
         -------
         Coordinate
             Coordinate at distance along Arc.
         """
+        if not distance and not fraction and not percentage:
+            raise Exception("You must provide either a distance, fraction or percentage.")
+
+        if distance and fraction:
+            warn("Both distance and fraction provided. Using distance.", UserWarning)
+        if distance and percentage:
+            warn("Both distance and percentage provided. Using distance.", UserWarning)
+
+        if not distance:
+            if fraction and percentage:
+                warn("Both fraction and percentage provided. Using fraction.", UserWarning)
+            if fraction:
+                distance = self.length * fraction
+            elif percentage:
+                distance = self.length * (percentage / 100)
+
         ref_coordinate_angle = atan2(
             (ref_coordinate.y - self.centre.y), (ref_coordinate.x - self.centre.x)
         )

diff --git a/tests/test_geometry.py b/tests/test_geometry.py
@@ -574,9 +574,56 @@ def test_line_get_coordinate_from_percentage_distance():
 def test_line_get_coordinate_from_distance():
     line = geometry.Line(geometry.Coordinate(0, 0), geometry.Coordinate(2, 0))
 
+    # test using the 'distance' argument
     assert line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1) == geometry.Coordinate(
         1, 0
     )
+    # test using the 'fraction' argument
+    assert line.get_coordinate_from_distance(
+        geometry.Coordinate(0, 0), fraction=0.5
+    ) == geometry.Coordinate(1, 0)
+    # test using the 'percentage' argument
+    assert line.get_coordinate_from_distance(
+        geometry.Coordinate(0, 0), percentage=50
+    ) == geometry.Coordinate(1, 0)
+
+    # test that warnings are raised when multiple arguments are given
+    # distance and fraction
+    with pytest.warns(UserWarning) as record:
+        coord = line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1, fraction=0.6)
+    assert "Both distance and fraction provided" in record[0].message.args[0]
+    # check that distance is used
+    assert coord == line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1)
+
+    # distance and percentage
+    with pytest.warns(UserWarning) as record:
+        coord = line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1, percentage=40)
+    assert "Both distance and percentage provided" in record[0].message.args[0]
+    # check that distance is used
+    assert coord == line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1)
+
+    # fraction and percentage
+    with pytest.warns(UserWarning) as record:
+        coord = line.get_coordinate_from_distance(
+            geometry.Coordinate(0, 0), fraction=0.6, percentage=40
+        )
+    assert "Both fraction and percentage provided" in record[0].message.args[0]
+    # check that fraction is used
+    assert coord == line.get_coordinate_from_distance(geometry.Coordinate(0, 0), fraction=0.6)
+
+    # distance, fraction and percentage
+    with pytest.warns(UserWarning) as record:
+        coord = line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1, 0.6, 40)
+    assert "Both distance and fraction provided" in record[0].message.args[0]
+    # check that both warnings are given
+    assert "Both distance and percentage provided" in record[1].message.args[0]
+    # check that distance is used
+    assert coord == line.get_coordinate_from_distance(geometry.Coordinate(0, 0), 1)
+
+    # neither distance, fraction or percentage are provided
+    with pytest.raises(Exception) as e_info:
+        coord = line.get_coordinate_from_distance(line.start)
+    assert "provide either a distance, fraction or percentage" in str(e_info)
 
 
 def test_line_length():
@@ -585,14 +632,14 @@ def test_line_length():
     assert line.length == sqrt(2)
 
 
-def test_arc_get_coordinate_from_percentage_distance():
+def test_arc_get_coordinate_from_fractional_distance():
     arc = geometry.Arc(
         geometry.Coordinate(-1, 0), geometry.Coordinate(1, 0), geometry.Coordinate(0, 0), 1
     )
 
-    coord = arc.get_coordinate_from_percentage_distance(geometry.Coordinate(-1, 0), 0.5)
-    assert isclose(coord.x, 0, abs_tol=1e-12)
-    assert isclose(coord.y, -1, abs_tol=1e-12)
+    coord_1 = arc.get_coordinate_from_percentage_distance(geometry.Coordinate(-1, 0), 0.5)
+    assert isclose(coord_1.x, 0, abs_tol=1e-12)
+    assert isclose(coord_1.y, -1, abs_tol=1e-12)
 
     # test an arc that failed with the old definition of get_coordinate_from_percentage_distance()
     arc_2 = geometry.Arc(geometry.Coordinate(62, 20), geometry.Coordinate(56, 33), radius=45)
@@ -604,32 +651,49 @@ def test_arc_get_coordinate_from_percentage_distance():
     assert math.isclose(arc_2.start.y, coord_3.y, abs_tol=1e-12)
     # test arc drawn clockwise
     arc_4 = geometry.Arc(geometry.Coordinate(56, 33), geometry.Coordinate(62, 20), radius=45)
-    coord_4 = arc_4.get_coordinate_from_distance(arc_4.end, 1e-13)
+    coord_4 = arc_4.get_coordinate_from_percentage_distance(arc_4.end, 1e-13)
     assert math.isclose(arc_4.end.x, coord_4.x, abs_tol=1e-12)
     assert math.isclose(arc_4.end.y, coord_4.y, abs_tol=1e-12)
-    coord_5 = arc_4.get_coordinate_from_distance(arc_4.start, 1e-13)
+    coord_5 = arc_4.get_coordinate_from_percentage_distance(arc_4.start, 1e-13)
     assert math.isclose(arc_4.start.x, coord_5.x, abs_tol=1e-12)
     assert math.isclose(arc_4.start.y, coord_5.y, abs_tol=1e-12)
+    # test arc with negative radius
+    arc_6 = geometry.Arc(
+        geometry.Coordinate(-1, 0), geometry.Coordinate(1, 0), geometry.Coordinate(0, 0), -1
+    )
+    coord_6 = arc_6.get_coordinate_from_percentage_distance(Coordinate(-1, 0), 0.5)
+    assert math.isclose(coord_6.x, 0, abs_tol=1e-12)
+    assert math.isclose(coord_6.y, 1, abs_tol=1e-12)
 
 
 def test_arc_get_coordinate_from_distance():
     arc = geometry.Arc(
         geometry.Coordinate(-1, 0), geometry.Coordinate(1, 0), geometry.Coordinate(0, 0), 1
     )
 
+    # test using the 'distance' argument
     coord = arc.get_coordinate_from_distance(geometry.Coordinate(-1, 0), math.pi / 2)
     assert math.isclose(coord.x, 0, abs_tol=1e-12)
     assert math.isclose(coord.y, -1, abs_tol=1e-12)
 
-    # test an arc that failed with the old definition of get_coordinate_from_distance()
+    # test for an arc with negative radius using the 'distance' argument
+    arc_1 = geometry.Arc(
+        geometry.Coordinate(-1, 0), geometry.Coordinate(1, 0), geometry.Coordinate(0, 0), -1
+    )
+    coord_1 = arc_1.get_coordinate_from_distance(geometry.Coordinate(-1, 0), math.pi / 2)
+    assert math.isclose(coord_1.x, 0, abs_tol=1e-12)
+    assert math.isclose(coord_1.y, 1, abs_tol=1e-12)
+
+    # test an arc that failed with the old definition of get_coordinate_from_distance() using the
+    # 'distance' argument
     arc_2 = geometry.Arc(geometry.Coordinate(62, 20), geometry.Coordinate(56, 33), radius=45)
     coord_2 = arc_2.get_coordinate_from_distance(arc_2.end, 1e-15)
     assert math.isclose(arc_2.end.x, coord_2.x, abs_tol=1e-12)
     assert math.isclose(arc_2.end.y, coord_2.y, abs_tol=1e-12)
     coord_3 = arc_2.get_coordinate_from_distance(arc_2.start, 1e-15)
     assert math.isclose(arc_2.start.x, coord_3.x, abs_tol=1e-12)
     assert math.isclose(arc_2.start.y, coord_3.y, abs_tol=1e-12)
-    # test arc drawn clockwise
+    # test arc drawn clockwise using the 'distance' argument
     arc_4 = geometry.Arc(geometry.Coordinate(56, 33), geometry.Coordinate(62, 20), radius=45)
     coord_4 = arc_4.get_coordinate_from_distance(arc_4.end, 1e-15)
     assert math.isclose(arc_4.end.x, coord_4.x, abs_tol=1e-12)
@@ -641,6 +705,48 @@ def test_arc_get_coordinate_from_distance():
     assert math.isclose(60.389142028418, coord_6.x, abs_tol=1e-12)
     assert math.isclose(24.730689908764, coord_6.y, abs_tol=1e-12)
 
+    # test using the 'fraction' argument
+    coord_7 = arc.get_coordinate_from_distance(geometry.Coordinate(-1, 0), fraction=0.5)
+    assert isclose(coord_7.x, 0, abs_tol=1e-12)
+    assert isclose(coord_7.y, -1, abs_tol=1e-12)
+
+    # test using the 'percentage' argument
+    coord_8 = arc.get_coordinate_from_distance(geometry.Coordinate(-1, 0), percentage=50)
+    assert isclose(coord_8.x, 0, abs_tol=1e-12)
+    assert isclose(coord_8.y, -1, abs_tol=1e-12)
+
+    # test that warnings are raised when multiple arguments are given
+    # distance and fraction
+    with pytest.warns(UserWarning) as record:
+        coord = arc.get_coordinate_from_distance(arc.start, 1, fraction=0.6)
+    assert "Both distance and fraction provided" in record[0].message.args[0]
+    # check that distance is used
+    assert coord == arc.get_coordinate_from_distance(arc.start, 1)
+    # distance and percentage
+    with pytest.warns(UserWarning) as record:
+        coord = arc.get_coordinate_from_distance(arc.start, 1, percentage=40)
+    assert "Both distance and percentage provided" in record[0].message.args[0]
+    # check that distance is used
+    assert coord == arc.get_coordinate_from_distance(arc.start, 1)
+    # fraction and percentage
+    with pytest.warns(UserWarning) as record:
+        coord = arc.get_coordinate_from_distance(arc.start, fraction=0.6, percentage=40)
+    assert "Both fraction and percentage provided" in record[0].message.args[0]
+    # check that fraction is used
+    assert coord == arc.get_coordinate_from_distance(arc.start, fraction=0.6)
+    # distance, fraction and percentage
+    with pytest.warns(UserWarning) as record:
+        coord = arc.get_coordinate_from_distance(arc.start, 1, 0.6, 40)
+    assert "Both distance and fraction provided" in record[0].message.args[0]
+    # check that both warnings are given
+    assert "Both distance and percentage provided" in record[1].message.args[0]
+    # check that distance is used
+    assert coord == arc.get_coordinate_from_distance(arc.start, 1)
+    # neither distance, fraction or percentage are provided
+    with pytest.raises(Exception) as e_info:
+        coord = arc.get_coordinate_from_distance(arc.start)
+    assert "provide either a distance, fraction or percentage" in str(e_info)
+
 
 def test_arc_length():
     arc = geometry.Arc(