Repository: koaning/scikit-lego
Pull request #710 (2024-11-01 19:19:36)

Monotonicspline
# Description

Fixed one thing in the implementation of `MonotonicSplineTransformer`. 
Added tests directly checking:
- monotonicity
- boundedness
- number of splines in the output.

Fixes #(issue)

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)


## Checklist:

- [x] My code follows the style guidelines (ruff)
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation (also to the readme.md)
- [x] I have added tests that prove my fix is effective or that my feature works
- [ ] I have added tests to check whether the new feature adheres to the sklearn convention
- [x] New and existing unit tests pass locally with my changes


Patch:
diff --git a/sklego/preprocessing/monotonicspline.py b/sklego/preprocessing/monotonicspline.py
index 86722cd..39638d1 100644
--- a/sklego/preprocessing/monotonicspline.py
+++ b/sklego/preprocessing/monotonicspline.py
@@ -61,12 +61,6 @@ class MonotonicSplineTransformer(TransformerMixin, BaseEstimator):
             )
             for col in range(X.shape[1])
         }
-        self.sorted_X = {col: np.sort(X[:, col]) for col in range(X.shape[1])}
-        self.sorted_X_output_ = {
-            col: self.spline_transformer_[col].transform(np.sort(X[:, col]).reshape(-1, 1)).cumsum(axis=0)
-            for col in range(X.shape[1])
-        }
-        self.sorted_idx_ = np.arange(X.shape[0]).astype(int)
         return self
 
     def transform(self, X):
@@ -95,6 +89,10 @@ class MonotonicSplineTransformer(TransformerMixin, BaseEstimator):
         )
         out = []
         for col in range(X.shape[1]):
-            mapping = np.interp(X[:, col], self.sorted_X[col], self.sorted_idx_).astype(int)
-            out.append(self.sorted_X_output_[col][mapping])
+            out.append(
+                np.cumsum(
+                    self.spline_transformer_[col].transform(X[:, [col]])[:, ::-1],
+                    axis=1,
+                )
+            )
         return np.concatenate(out, axis=1)


Test patch:
diff --git a/tests/test_preprocessing/test_monospline.py b/tests/test_preprocessing/test_monospline.py
index 1d7bcab..85533f8 100644
--- a/tests/test_preprocessing/test_monospline.py
+++ b/tests/test_preprocessing/test_monospline.py
@@ -10,7 +10,9 @@ from sklego.preprocessing import MonotonicSplineTransformer
 @pytest.mark.parametrize("knots", ["uniform", "quantile"])
 def test_monotonic_spline_transformer(n_knots, degree, knots):
     X = np.random.uniform(size=(100, 10))
-    transformer = MonotonicSplineTransformer(n_knots=n_knots, degree=degree, knots=knots)
+    transformer = MonotonicSplineTransformer(
+        n_knots=n_knots, degree=degree, knots=knots
+    )
     transformer_sk = SplineTransformer(n_knots=n_knots, degree=degree, knots=knots)
     transformer.fit(X)
     transformer_sk.fit(X)
@@ -21,8 +23,23 @@ def test_monotonic_spline_transformer(n_knots, degree, knots):
     # Both should have the same shape
     assert out.shape == out_sk.shape
 
-    # Check that the monotonic variant always has a higher value than the non-monotonic variant
-    for col in range(out.shape[1]):
-        col_values = out[:, col]
-        col_values_sk = out_sk[:, col]
-        assert np.all(col_values >= col_values_sk), f"Column {col} is not monotonically increasing"
+    n_splines_per_feature = n_knots + degree - 1
+    assert out.shape[1] == X.shape[1] * n_splines_per_feature
+
+    # I splines should be bounded by 0 and 1
+    assert np.logical_or(out >= 0, np.isclose(out, 0)).all()
+    assert np.logical_or(out <= 1, np.isclose(out, 1)).all()
+
+    # The features should be monotonically increasing
+    for i in range(X.shape[1]):
+        feature = X[:, i]
+        sorted_out = out[
+            np.argsort(feature),
+            i * n_splines_per_feature : (i + 1) * n_splines_per_feature
+        ]
+        differences = np.diff(sorted_out, axis=0)
+
+        # All differences should be greater or equal to zero upto floating point errors
+        assert np.logical_or(
+            np.greater_equal(differences, 0), np.isclose(differences, 0)
+        ).all()