Dessia-tech / volmdlr

A python VOLume MoDeLeR computations-oriented with STEP support for import/export
GNU Lesser General Public License v2.1
26 stars 10 forks source link

Method `from_3_points` failing when called with FullArc2D cls #1281

Open GhislainJ opened 7 months ago

GhislainJ commented 7 months ago

Status is high because it blocks the reference_path PR

Given the following error :

image

from volmdlr.edges import FullArc2D

p0 = Point2D(0, 0)
p1 = Point2D(0, 1)
p2 = Point2D(1, 1) 
a = FullArc2D.from_3_points(p0, p1, p2)

I unsure if this there is an expected behavior, but i guess this should not fail as it is covered by the test surfaces/test_cylindrical_surface3d.TestCylindricalSurface3D.test_contour3d_to_2d

This is problematic because the test failed when I added some argument to the init functions which misaligned the two signatures :

class Arc2D(ArcMixin, Edge):
    def __init__(self, circle: 'volmdlr.curves.Circle2D', start: volmdlr.Point2D, end: volmdlr.Point2D, reference_path: str = PATH_ROOT, name: str = ''):
class FullArc2D(FullArcMixin, Arc2D):
    def __init__(self, circle: 'volmdlr.curves.Circle2D', start_end: volmdlr.Point2D, reference_path: str = PATH_ROOT, name: str = ''):

The following call can have Arc2D or FullArc2D as cls :

arc = cls(circle, point1, point3, reference_path, name)

It means that before that, we were able to generate an Arc2D as well as a FullArc2D, but by giving respectively point1 to FullArc2D's start_end and point3 to reference_path.

I could make it work by removing either the reference_path or name argument (as it was done before), but this is plainly wrong

Either override FullArc2D's method or prevent user from creating a FullArc2D from 3 points. I am going to propose a fix with the former, here