TST improve test-coverage in base, remove unreachable code-path

Author: amueller

sklearn/base.py

@@ -196,6 +196,7 @@ def get_params(self, deep=True):
         out = dict()
         for key in self._get_param_names():
             value = getattr(self, key, None)
+            # XXX: should we rather test if instance of estimator?
             if deep and hasattr(value, 'get_params'):
                 deep_items = value.get_params().items()
                 out.update((key + '__' + k, val) for k, val in deep_items)
@@ -227,12 +228,6 @@ def set_params(self, **params):
                     raise ValueError('Invalid parameter %s for estimator %s'
                             % (name, self))
                 sub_object = valid_params[name]
-                if not hasattr(sub_object, 'get_params'):
-                    raise TypeError(
-                    'Parameter %s of %s is not an estimator, cannot set '
-                    'sub parameter %s' %
-                        (sub_name, self.__class__.__name__, sub_name)
-                    )
                 sub_object.set_params(**{sub_name: value})
             else:
                 # simple objects case

sklearn/tests/test_base.py

@@ -44,6 +44,23 @@ def __init__(self, a=None):
         self.a = 1
 
 
+class NoEstimator(object):
+    def __init__(self):
+        pass
+
+    def fit(self, X=None, y=None):
+        return self
+
+    def predict(self, X=None):
+        return None
+
+
+class VargEstimator(BaseEstimator):
+    """Sklearn estimators shouldn't have vargs."""
+    def __init__(self, *vargs):
+        pass
+
+
 #############################################################################
 # The tests
 
@@ -88,6 +105,12 @@ def test_clone_buggy():
     buggy.a = 2
     assert_raises(RuntimeError, clone, buggy)
 
+    no_estimator = NoEstimator()
+    assert_raises(TypeError, clone, no_estimator)
+
+    varg_est = VargEstimator()
+    assert_raises(RuntimeError, clone, varg_est)
+
 
 def test_clone_empty_array():
     """Regression test for cloning estimators with empty arrays"""
@@ -110,6 +133,9 @@ def test_repr():
         "T(a=K(c=None, d=None), b=K(c=None, d=None))"
     )
 
+    some_est = T(a=["long_params"] * 1000)
+    assert_equal(len(repr(some_est)), 415)
+
 
 def test_str():
     """Smoke test the str of the base estimator"""
@@ -135,3 +161,16 @@ def test_is_classifier():
     assert_true(is_classifier(Pipeline([('svc', svc)])))
     assert_true(is_classifier(Pipeline([('svc_cv',
                               GridSearchCV(svc, {'C': [0.1, 1]}))])))
+
+
+def test_set_params():
+    # test nested estimator parameter setting
+    clf = Pipeline([("svc", SVC())])
+    # non-existing parameter in svc
+    assert_raises(ValueError, clf.set_params, svc__stupid_param=True)
+    # non-existing parameter of pipeline
+    assert_raises(ValueError, clf.set_params, svm__stupid_param=True)
+    # we don't currently catch if the things in pipeline are estimators
+    #bad_pipeline = Pipeline([("bad", NoEstimator())])
+    #assert_raises(AttributeError, bad_pipeline.set_params,
+            #bad__stupid_param=True)