Skip to content

Commit

Permalink
[MRG+1] Don't modify steps in {Pipeline,FeatureUnion}.__init__ (#9716)
Browse files Browse the repository at this point in the history
  • Loading branch information
amueller authored and jnothman committed Sep 12, 2017
1 parent 7d6f307 commit 81fb483
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
8 changes: 5 additions & 3 deletions sklearn/pipeline.py
Expand Up @@ -110,8 +110,7 @@ class Pipeline(_BaseComposition):
# BaseEstimator interface

def __init__(self, steps, memory=None):
# shallow copy of steps
self.steps = list(steps)
self.steps = steps
self._validate_steps()
self.memory = memory

Expand Down Expand Up @@ -184,6 +183,8 @@ def _final_estimator(self):
# Estimator interface

def _fit(self, X, y=None, **fit_params):
# shallow copy of steps - this should really be steps_
self.steps = list(self.steps)
self._validate_steps()
# Setup the memory
memory = check_memory(self.memory)
Expand Down Expand Up @@ -613,7 +614,7 @@ class FeatureUnion(_BaseComposition, TransformerMixin):
"""
def __init__(self, transformer_list, n_jobs=1, transformer_weights=None):
self.transformer_list = list(transformer_list)
self.transformer_list = transformer_list
self.n_jobs = n_jobs
self.transformer_weights = transformer_weights
self._validate_transformers()
Expand Down Expand Up @@ -704,6 +705,7 @@ def fit(self, X, y=None):
self : FeatureUnion
This estimator
"""
self.transformer_list = list(self.transformer_list)
self._validate_transformers()
transformers = Parallel(n_jobs=self.n_jobs)(
delayed(_fit_one_transformer)(trans, X, y)
Expand Down
7 changes: 6 additions & 1 deletion sklearn/tests/test_pipeline.py
Expand Up @@ -19,6 +19,7 @@
from sklearn.utils.testing import assert_array_equal
from sklearn.utils.testing import assert_array_almost_equal
from sklearn.utils.testing import assert_dict_equal
from sklearn.utils.testing import assert_no_warnings

from sklearn.base import clone, BaseEstimator
from sklearn.pipeline import Pipeline, FeatureUnion, make_pipeline, make_union
Expand Down Expand Up @@ -187,7 +188,7 @@ def test_pipeline_init():
assert_raises(ValueError, pipe.set_params, anova__C=0.1)

# Test clone
pipe2 = clone(pipe)
pipe2 = assert_no_warnings(clone, pipe)
assert_false(pipe.named_steps['svc'] is pipe2.named_steps['svc'])

# Check that apart from estimators, the parameters are the same
Expand Down Expand Up @@ -421,6 +422,10 @@ def test_feature_union():
X_sp_transformed = fs.fit_transform(X_sp, y)
assert_array_almost_equal(X_transformed, X_sp_transformed.toarray())

# Test clone
fs2 = assert_no_warnings(clone, fs)
assert_false(fs.transformer_list[0][1] is fs2.transformer_list[0][1])

# test setting parameters
fs.set_params(select__k=2)
assert_equal(fs.fit_transform(X, y).shape, (X.shape[0], 4))
Expand Down

0 comments on commit 81fb483

Please sign in to comment.