ukaea / paramak

Create parametric 3D fusion reactor CAD models
https://paramak.readthedocs.io/en/main/
37 stars 12 forks source link

points are not reusable between Shapes #751

Open shimwell opened 3 years ago

shimwell commented 3 years ago

Points is passed by the user and changed internally by the class (perhaps this is poor program design).

One change adds the start point as the end point to make a closed wire. This is nice as it saves the user time and having to make that last point which the program can easily figure out.

It also checks that the input first and last points are not the same.

Anyway the points is pass as a list which is mutable.

Therefore the list values are changed and can't be reused. This simple example produces an error.

Perhaps we should not change the points that the user inputs by make a copy and change the copy?

points =[
        (100, 0, "straight"),
        (200, 0, "straight"),
        (250, 50, "straight"),
        (200, 100, "straight"),
    ]

test_rotate_mixed_shape = paramak.RotateMixedShape(
    rotation_angle=180,
    points=points
)
test_extrude_mixed_shape = paramak.ExtrudeMixedShape(
    distance=10,
    points=points
)
ValueError                                Traceback (most recent call last)
<ipython-input-9-eeb600e00657> in <module>
     10     points=points
     11 )
---> 12 test_extrude_mixed_shape = paramak.ExtrudeMixedShape(
     13     distance=10,
     14     points=points

~/paramak/paramak/parametric_shapes/extruded_mixed_shape.py in __init__(self, distance, extrude_both, rotation_angle, extrusion_start_offset, stp_filename, stl_filename, **kwargs)
     33     ):
     34 
---> 35         super().__init__(
     36             stp_filename=stp_filename,
     37             stl_filename=stl_filename,

~/paramak/paramak/shape.py in __init__(self, points, connection_type, name, color, material_tag, stp_filename, stl_filename, azimuth_placement_angle, workplane, rotation_axis, tet_mesh, surface_reflectivity, physical_groups, cut, intersect, union)
     94 
     95         self.connection_type = connection_type
---> 96         self.points = points
     97         self.stp_filename = stp_filename
     98         self.stl_filename = stl_filename

~/paramak/paramak/shape.py in points(self, values)
    465                     msg = "The coordinates of the last and first points are \
    466                         the same."
--> 467                     raise ValueError(msg)
    468 
    469                 values.append(values[0])

ValueError: The coordinates of the last and first points are                         the same.
RemDelaporteMathurin commented 3 years ago

I noticed that a while ago, and it was a tad annoying 😸

shimwell commented 3 years ago

Another related problem is that if one wants to reuse the points in another shape then I think they will have been modified by the first shape and potentially unusable in the second shape

shape1 = paramak.RotateStraightShape(
    points= [
    (100, 0),
    (200, 0),
    (250, 50),
    (200, 100),
]
)

# shape1.points is now a list with connections not what the user originally input

shape1 = paramak.RotateSplineShape(
    points= shape1.points
]
)
RemDelaporteMathurin commented 3 years ago

One way to solve this would be to have two separate attributes points and connections.

One would only contain coordinates and the latter will only contain strings?

shimwell commented 3 years ago

Or perhaps another version of points called processed_points or something like that

RemDelaporteMathurin commented 3 years ago

@Shimwell I think this can be closed since it was solved in #839

shimwell commented 3 years ago

Do you mind if I keep it open a bit longer, I need to get that example I posted a few comments up working