Merge pull request #178 from lucasimi/bugfix/nested-params

Fixed clone

Author: lucasimi

src/tdamapper/_common.py

@@ -126,17 +126,18 @@ def set_params(self, **params):
         return self
 
     def __repr__(self):
-        obj = type(self)()
-        rep = f'{self.__class__.__name__}('
+        obj_noargs = type(self)()
+        args_repr = []
         for k, v in self.__dict__.items():
-            obj_v = getattr(obj, k)
-            if self.__is_param_public(k) and not v == obj_v:
-                rep += f'{k}={v}, '
-        rep += ')'
-        return rep
+            v_default = getattr(obj_noargs, k)
+            v_default_repr = repr(v_default)
+            v_repr = repr(v)
+            if self.__is_param_public(k) and not v_repr == v_default_repr:
+                args_repr.append(f'{k}={v_repr}')
+        return f"{self.__class__.__name__}({', '.join(args_repr)})"
 
 
-def clone(estimator):
+def clone(obj):
     """
     Clone an estimator, returning a new one, unfitted, having the same public
     parameters.
@@ -146,5 +147,7 @@ def clone(estimator):
     :return: A new estimator with the same parameters.
     :rtype: A scikit-learn compatible estimator
     """
-    params = estimator.get_params(deep=True)
-    return type(estimator)(**params)
+    params = obj.get_params(deep=True)
+    obj_noargs = type(obj)()
+    obj_noargs.set_params(**params)
+    return obj_noargs

src/tdamapper/cover.py

@@ -512,24 +512,20 @@ def __init__(
         self.leaf_capacity = leaf_capacity
         self.leaf_radius = leaf_radius
         self.pivoting = pivoting
-        if algorithm == 'proximity':
-            self.__cubical_cover = ProximityCubicalCover(
-                n_intervals=n_intervals,
-                overlap_frac=overlap_frac,
-                kind=kind,
-                leaf_capacity=leaf_capacity,
-                leaf_radius=leaf_radius,
-                pivoting=pivoting,
-            )
-        elif algorithm == 'standard':
-            self.__cubical_cover = StandardCubicalCover(
-                n_intervals=n_intervals,
-                overlap_frac=overlap_frac,
-                kind=kind,
-                leaf_capacity=leaf_capacity,
-                leaf_radius=leaf_radius,
-                pivoting=pivoting,
-            )
+
+    def __get_cubical_cover(self):
+        params = dict(
+            n_intervals=self.n_intervals,
+            overlap_frac=self.overlap_frac,
+            kind=self.kind,
+            leaf_capacity=self.leaf_capacity,
+            leaf_radius=self.leaf_radius,
+            pivoting=self.pivoting,
+        )
+        if self.algorithm == 'proximity':
+            return ProximityCubicalCover(**params)
+        elif self.algorithm == 'standard':
+            return StandardCubicalCover(**params)
         else:
             raise ValueError(
                 "The only possible values for algorithm are 'standard' and "
@@ -548,7 +544,9 @@ def fit(self, X):
         :return: The object itself.
         :rtype: self
         """
-        return self.__cubical_cover.fit(X)
+        self.__cubical_cover = self.__get_cubical_cover()
+        self.__cubical_cover.fit(X)
+        return self
 
     def search(self, x):
         """
@@ -576,4 +574,5 @@ def apply(self, X):
         :return: A generator of lists of ids.
         :rtype: generator of lists of ints
         """
+        self.__cubical_cover = self.__get_cubical_cover()
         return self.__cubical_cover.apply(X)

tests/test_unit_core.py

@@ -10,7 +10,7 @@
     mapper_labels,
     TrivialCover,
 )
-from tdamapper.cover import BallCover
+from tdamapper.cover import BallCover, CubicalCover, ProximityCubicalCover, StandardCubicalCover
 from tdamapper.clustering import TrivialClustering
 
 
@@ -80,7 +80,7 @@ def test_ball_large_radius(self):
         ccs2 = mapper_connected_components(data, data, cover, clustering)
         self.assertEqual(len(data), len(ccs2))
 
-    def test_two_disconnected_clusters(self):
+    def test_ball_two_disconnected_clusters(self):
         data = [np.array([float(i), 0.0]) for i in range(100)]
         data.extend([np.array([float(i), 500.0]) for i in range(100)])
         data = np.array(data)
@@ -98,7 +98,7 @@ def test_two_disconnected_clusters(self):
         ccs2 = mapper_connected_components(data, data, cover, clustering)
         self.assertEqual(len(data), len(ccs2))
 
-    def test_two_connected_clusters(self):
+    def test_ball_two_connected_clusters(self):
         data = [
             np.array([0.0, 1.0]), np.array([1.0, 0.0]),
             np.array([0.0, 0.0]), np.array([1.0, 1.0])]
@@ -116,7 +116,7 @@ def test_two_connected_clusters(self):
         ccs2 = mapper_connected_components(data, data, cover, clustering)
         self.assertEqual(len(data), len(ccs2))
 
-    def test_two_connected_clusters_parallel(self):
+    def test_ball_two_connected_clusters_parallel(self):
         data = [
             np.array([0.0, 1.0]), np.array([1.0, 0.0]),
             np.array([0.0, 0.0]), np.array([1.0, 1.0])]
@@ -136,7 +136,23 @@ def test_two_connected_clusters_parallel(self):
         ccs2 = mapper_connected_components(data, data, cover, clustering)
         self.assertEqual(len(data), len(ccs2))
 
-    def test_connected_components(self):
+    def test_proximity_cubical_line(self):
+        data = np.array([[float(i)] for i in range(1000)])
+        cover = ProximityCubicalCover(n_intervals=4, overlap_frac=0.5)
+        clustering = TrivialClustering()
+        mp = MapperAlgorithm(cover, clustering)
+        g = mp.fit_transform(data, data)
+        self.assertEqual(4, len(g.nodes))
+
+    def test_cubical_line(self):
+        data = np.array([[float(i)] for i in range(1000)])
+        cover = CubicalCover(n_intervals=4, overlap_frac=0.5)
+        clustering = TrivialClustering()
+        mp = MapperAlgorithm(cover, clustering)
+        g = mp.fit_transform(data, data)
+        self.assertEqual(4, len(g.nodes))
+
+    def test_mock_connected_components(self):
         data = [0, 1, 2, 3]
 
         class MockCover:
@@ -156,7 +172,7 @@ def apply(self, X):
         self.assertEqual(cc0, ccs[2])
         self.assertEqual(cc0, ccs[3])
 
-    def test_labels(self):
+    def test_mock_labels(self):
         data = [0, 1, 2, 3]
 
         class MockCover:

tests/test_unit_params.py

@@ -1,5 +1,8 @@
 import unittest
 
+from sklearn.cluster import DBSCAN
+
+from tdamapper._common import clone
 from tdamapper.core import MapperAlgorithm
 from tdamapper.cover import (
     BallCover,
@@ -11,7 +14,23 @@
 
 class TestParams(unittest.TestCase):
 
-    def test_params_mapper(self):
+    def __test_clone(self, obj):
+        obj_repr = repr(obj)
+        obj_cln = clone(obj)
+        cln_repr = repr(obj_cln)
+        self.assertEquals(obj_repr, cln_repr)
+
+    def __test_repr(self, obj):
+        obj_repr = repr(obj)
+        _obj = eval(obj_repr)
+        _obj_repr = repr(_obj)
+        self.assertEquals(obj_repr, _obj_repr)
+
+    def __test_clone_and_repr(self, obj):
+        self.__test_clone(obj)
+        self.__test_repr(obj)
+
+    def test_params_mapper_algorithm(self):
         est = MapperAlgorithm(
             cover=CubicalCover(
                 n_intervals=3,
@@ -30,7 +49,7 @@ def test_params_mapper(self):
         self.assertEquals(2, params['cover__n_intervals'])
         self.assertEquals(0.2, params['cover__overlap_frac'])
 
-    def test_params_clust(self):
+    def test_params_mapper_clustering(self):
         est = MapperClustering(
             cover=CubicalCover(
                 n_intervals=3,
@@ -47,4 +66,58 @@ def test_params_clust(self):
         params = est.get_params()
         self.assertEquals(10, len(params))
         self.assertEquals(2, params['cover__n_intervals'])
-        self.assertEquals(0.2, params['cover__overlap_frac'])
+        self.assertEquals(0.2, params['cover__overlap_frac'])
+
+    def test_clone_and_repr_ball_cover(self):
+        self.__test_clone_and_repr(BallCover())
+        self.__test_clone_and_repr(BallCover(
+            radius=2.0,
+            metric='test',
+            metric_params={'f': 4},
+            kind='kind_test',
+            leaf_capacity=3.0,
+            leaf_radius=-2.0,
+            pivoting=7,
+        ))
+
+    def test_clone_and_repr_cubical_cover(self):
+        self.__test_clone_and_repr(CubicalCover())
+        self.__test_clone_and_repr(CubicalCover(
+            n_intervals=4,
+            overlap_frac=5,
+            algorithm='algo_test',
+            kind='simple',
+            leaf_radius=5,
+            leaf_capacity=6,
+            pivoting='no'
+        ))
+
+    def test_clone_repr_mapper_algorithm(self):
+        self.__test_clone_and_repr(MapperAlgorithm())
+        self.__test_clone_and_repr(MapperAlgorithm(
+            cover=CubicalCover(
+                n_intervals=3,
+                overlap_frac=0.3,
+            ),
+            clustering=DBSCAN(
+                eps='none',
+                min_samples=5.4,
+            ),
+            failsafe=4,
+            n_jobs='foo',
+            verbose=4,
+        ))
+
+    def test_clone_repr_mapper_clustering(self):
+        self.__test_clone_and_repr(MapperClustering())
+        self.__test_clone_and_repr(MapperClustering(
+            cover=CubicalCover(
+                n_intervals=3,
+                overlap_frac=0.3,
+            ),
+            clustering=DBSCAN(
+                eps='none',
+                min_samples=5.4,
+            ),
+            n_jobs='foo',
+        ))