Simplify the shape.py points setter

[shimwell]

Currently the setter in the shape.py checks quite a lot of things about the points

paramak/paramak/shape.py

Lines 412 to 471 in 785c3ed

	@points.setter
	def points(self, values):
	if values is not None:
	if not isinstance(values, list):
	raise ValueError("points must be a list")
	if self.connection_type != "mixed":
	values = [(*p, self.connection_type) for p in values]
	for value in values:
	if type(value) not in [list, tuple]:
	msg = "individual points must be a list or a tuple." + \
	"{} in of type {}".format(value, type(value))
	raise ValueError(msg)
	for value in values:
	# Checks that the length of each tuple in points is 2 or 3
	if len(value) not in [2, 3]:
	msg = "individual points contain 2 or 3 entries {} has a \
	length of {}".format(value, len(values[0]))
	raise ValueError(msg)
	# Checks that the XY points are numbers
	if not isinstance(value[0], numbers.Number):
	msg = "The first value in the tuples that make \
	up the points represents the X value \
	and must be a number {}".format(value)
	raise ValueError(msg)
	if not isinstance(value[1], numbers.Number):
	msg = "The second value in the tuples that make \
	up the points represents the X value \
	and must be a number {}".format(value)
	raise ValueError(msg)
	# Checks that only straight and spline are in the connections
	# part of points
	if len(value) == 3:
	if value[2] not in ["straight", "spline", "circle"]:
	msg = 'individual connections must be either \
	"straight", "circle" or "spline"'
	raise ValueError(msg)
	# checks that the entries in the points are either all 2 long or
	# all 3 long, not a mixture
	if not all(len(entry) == 2 for entry in values):
	if not all(len(entry) == 3 for entry in values):
	msg = "The points list should contain entries of length 2 \
	or 3 but not a mixture of 2 and 3"
	raise ValueError(msg)
	if len(values) > 1:
	if values[0][:2] == values[-1][:2]:
	msg = "The coordinates of the last and first points are \
	the same."
	raise ValueError(msg)
	values.append(values[0])
	self._points = values

This should be simplified and some of the logica can be moved into parametric shape classes that inherit from shape.

For example this could be added to the RotateStraightShape.py as a new setter that makes use of the existing points setter in Shape.py but also extends it

    @RotateMixedShape.points.setter
    def points(self, values):
        if values is not None:
            # Checks that the length of each tuple in points is 2
            for value in values:
                if len(value) != 2:
                    msg = "individual points must contain 2 coordinates {} has a \
                        length of {}".format(value, len(values[0]))
                    raise ValueError(msg) 

        super(RotateStraightShape, self.__class__).points.fset(self, values)

[RemDelaporteMathurin]

@shimwell just to be clear, in the snipset you posted, shouldn't the test for MixedShape points ensure values is a list of list of length 3 (X, Y, connection)?

[RemDelaporteMathurin]

Additionnaly, this would require to rewrite the @Property as well:
Example in RotateMixedShape

    @property
    def points(self):
        return super().points

    @points.setter
    def points(self, values_in):
        super(RotateMixedShape, self.__class__).points.fset(self, values_in)

[shimwell]

@shimwell just to be clear, in the snipset you posted, shouldn't the test for MixedShape points ensure values is a list of list of length 3 (X, Y, connection)?

ah yes looks like I got the names mixed up. The general idea is that we could move some checking to other classes which add extra checks to the check in the Shape.py class. These extra checks would be run and then the basic checks from Shape.py are also run via the super call

[shimwell]

I am not sure if this is worth the effort as I think in general it would increase the lines of code but perhaps simplify the code a bit

[RemDelaporteMathurin]

Yes some of the checks apply to all parametric shapes (hence why they were in Shape in the first place)