Repository: EPFL-Center-for-Imaging/splinebox

Issue:
Protect spline attributes
Some attributes of a spline object, such as `closed` or `basis_function`, can be modified after the spline has been created. However, this can be problematic if the control points have already been set. Modifying these attributes should either be restricted or require verification to ensure compatibility with the existing control points.

Padding differences are particularly challenging, especially when the new basis function has a different support size or when switching between open and closed splines.

Discussion:

Solution patch:
diff --git a/src/splinebox/spline_curves.py b/src/splinebox/spline_curves.py
index 9353500..945a3d4 100644
--- a/src/splinebox/spline_curves.py
+++ b/src/splinebox/spline_curves.py
@@ -68,10 +68,10 @@ def __init__(self, M, basis_function, closed=False, control_points=None, padding
             raise RuntimeError("M must be greater or equal than the spline generator support size.")
 
         self.basis_function = basis_function
-        self.halfSupport = self.basis_function.support / 2
+        self._half_support = self.basis_function.support / 2
         # Number of additional knots used for padding the ends
         # of an open spline
-        self.pad = math.ceil(self.halfSupport) - 1
+        self._pad = math.ceil(self._half_support) - 1
         self.closed = closed
         self.control_points = control_points
         self.padding_function = padding_function
@@ -186,6 +186,50 @@ def basis_function(self, value):
             )
         self._basis_function = value
 
+    @property
+    def M(self):
+        return self._M
+
+    @M.setter
+    def M(self, M):
+        if hasattr(self, "control_points") and self.control_points is not None and M != self.M:
+            # The has attribute is necessary because M is assigned before control_points in the constructor
+            raise RuntimeError(
+                "M cannot be changed after the control points were set. Create a new spline or set the control_points to None first."
+            )
+        self._M = M
+
+    @property
+    def closed(self):
+        return self._closed
+
+    @closed.setter
+    def closed(self, closed):
+        if hasattr(self, "control_points") and self.control_points is not None and closed != self.closed:
+            # The has attribute is necessary because closed is assigned before control_points in the constructor
+            raise RuntimeError(
+                "closed cannot be changed after the control points were set. Create a new spline or set the control_points to None first."
+            )
+        self._closed = closed
+
+    @property
+    def half_support(self):
+        return self._half_support
+
+    @half_support.setter
+    def half_support(self, _):
+        raise RuntimeError("The half support is determined by the basis function and cannot be set by the user.")
+
+    @property
+    def pad(self):
+        return self._pad
+
+    @pad.setter
+    def pad(self, _):
+        raise RuntimeError(
+            "The amount of necessary padding is automatically calculated based on the support of the basis function and cannot be changed."
+        )
+
     def copy(self):
         """
         Returns a deep copy of this spline.
@@ -812,7 +856,7 @@ def _get_tval(self, t):
             tval = np.full((len(t), len(k)), np.nan)
             # compute the positions at which the basis functions have to be evaluated
             # and save them in tval
-            self._wrap_index(t, k, self.halfSupport, self.M, tval)
+            self._wrap_index(t, k, self.half_support, self.M, tval)
         else:
             # take into account the padding with additional basis functions
             # for non-closed splines
@@ -827,25 +871,25 @@ def _get_tval(self, t):
     @numba.guvectorize(
         [(numba.float64[:], numba.float64[:], numba.float64, numba.int64, numba.float64[:, :])], "(n),(m),(),()->(n,m)"
     )
-    def _wrap_index(ts, ks, halfSupport, M, wrapped_tval):  # pragma: no cover
+    def _wrap_index(ts, ks, half_support, M, wrapped_tval):  # pragma: no cover
         """
         Fill the wrapped_tval array whenever a value t
         is affected by the basis function at knot k, taking
         into account that basis functions at the begging/end
         affect positions on the opposite end for closed splines.
         """
-        outside_tvalue = halfSupport + 1
+        outside_tvalue = half_support + 1
         for i, k in enumerate(ks):
             for j, t in enumerate(ts):
-                if t >= M + k - halfSupport:
+                if t >= M + k - half_support:
                     # t is close enough to the end to be affected by
                     # the k-th basis function from the beginning
                     wrapped_tval[j, i] = t - M - k
-                elif t <= halfSupport - (M - k):
+                elif t <= half_support - (M - k):
                     # t is close enough to the beginning to be affected
                     # by the k-th basis function, which is close to the end
                     wrapped_tval[j, i] = t + M - k
-                elif t > k + halfSupport or t < k - halfSupport:
+                elif t > k + half_support or t < k - half_support:
                     # t is outside the support of the k-th basis function
                     # this can be any value outside the support
                     wrapped_tval[j, i] = outside_tvalue


Test patch:
diff --git a/tests/test_spline_curves.py b/tests/test_spline_curves.py
index 599c716..e1f78ce 100644
--- a/tests/test_spline_curves.py
+++ b/tests/test_spline_curves.py
@@ -5,7 +5,7 @@
 import numpy as np
 import pytest
 import scipy
-import splinebox.basis_functions
+import splinebox
 
 
 def test_check_control_points(non_hermite_spline_curve, coeff_gen):
@@ -124,37 +124,37 @@ def test_closed_splines(closed_spline_curve, derivative, coeff_gen, is_hermite_s
     )
 
 
-def test_draw():
-    spline = splinebox.spline_curves.Spline(M=4, basis_function=splinebox.basis_functions.B1(), closed=True)
+def test_draw(closed):
+    spline = splinebox.spline_curves.Spline(M=4, basis_function=splinebox.basis_functions.B1(), closed=closed)
     spline._check_control_points = unittest.mock.MagicMock()
     spline.knots = np.array([[1, 1], [1, 2], [2, 2], [2, 1]])
 
     x = np.linspace(0, 3, 31)
     y = np.linspace(0, 3, 31)
 
-    expected = np.zeros((len(x), len(y)))
-    expected[11:20, 11:20] = 1
-    expected[10, 10:21] = 0.5
-    expected[20, 10:21] = 0.5
-    expected[10:21, 10] = 0.5
-    expected[10:21, 20] = 0.5
-
-    output = spline.draw(x, y)
-    assert np.allclose(output, expected)
+    # You can only draw closed splines
+    if not closed:
+        with pytest.raises(RuntimeError):
+            spline.draw(x, y)
+    else:
+        expected = np.zeros((len(x), len(y)))
+        expected[11:20, 11:20] = 1
+        expected[10, 10:21] = 0.5
+        expected[20, 10:21] = 0.5
+        expected[10:21, 10] = 0.5
+        expected[10:21, 20] = 0.5
 
-    # Check that the presence of the coefficients was verified
-    spline._check_control_points.assert_called()
+        output = spline.draw(x, y)
+        assert np.allclose(output, expected)
 
-    # You can only draw closed splines
-    spline.closed = False
-    with pytest.raises(RuntimeError):
-        spline.draw(x, y)
+        # Check that the presence of the coefficients was verified
+        spline._check_control_points.assert_called()
 
-    # You can only draw splines in 2D
-    spline = splinebox.spline_curves.Spline(M=4, basis_function=splinebox.basis_functions.B1(), closed=True)
-    spline.knots = np.array([[1, 1, 0.5], [1, 2, 2], [2, 2, 1], [2, 1, 0]])
-    with pytest.raises(RuntimeError):
-        spline.draw(x, y)
+        # You can only draw splines in 2D
+        spline = splinebox.spline_curves.Spline(M=4, basis_function=splinebox.basis_functions.B1(), closed=closed)
+        spline.knots = np.array([[1, 1, 0.5], [1, 2, 2], [2, 2, 1], [2, 1, 0]])
+        with pytest.raises(RuntimeError):
+            spline.draw(x, y)
 
 
 def test_is_inside():
@@ -891,3 +891,42 @@ def test_mesh(
             elif cap_ends:
                 # spherical polyhedra have Euler characteristic 2
                 assert V + F - E == 2
+
+
+def test_protected_spline_attributes(spline_curve, coeff_gen, basis_function):
+    # spline = splinebox.spline_curves.Spline(M=4, basis_function=splinebox.basis_functions.B1(), closed=True)
+    spline = spline_curve
+
+    # Test that all attributes can be changed before the control points are set
+    spline.M = spline.M + 1
+    spline.basis_function = basis_function
+    spline.closed = not spline.closed
+
+    # half_support and pad can never be changed
+    with pytest.raises(RuntimeError):
+        spline.half_support = spline.half_support + 1
+    with pytest.raises(RuntimeError):
+        spline.pad = spline.pad + 1
+
+    spline.control_points = coeff_gen(M=spline.M, support=spline.basis_function.support, closed=spline.closed)
+
+    # Check that we cannot change M anymore
+    with pytest.raises(RuntimeError):
+        spline.M = spline.M + 2
+
+    # Check that basis_function can only be changed to basis functions with the same support
+    if basis_function.support != spline.basis_function.support:
+        with pytest.raises(RuntimeError):
+            spline.basis_function = basis_function
+    else:
+        spline.basis_function = basis_function
+
+    # Check that closed cannot be changed anymore
+    with pytest.raises(RuntimeError):
+        spline.closed = not spline.closed
+
+    # half_support and pad can be changed after setting the control points either
+    with pytest.raises(RuntimeError):
+        spline.half_support = spline.half_support + 1
+    with pytest.raises(RuntimeError):
+        spline.pad = spline.pad + 1