force pipeline steps to be list not a tuple #9221
Conversation
| @@ -145,6 +145,9 @@ def set_params(self, **kwargs): | |||
| return self | |||
|
|
|||
| def _validate_steps(self): | |||
| if isinstance(self.steps, tuple): | |||
| self.steps = list(self.steps) | |||
There was a problem hiding this comment.
Modifying an init argument :(
There was a problem hiding this comment.
what do you suggest?
it is modified anyway in : L226
self.steps[step_idx] = (name, fitted_transformer)
which is what created the problem
|
Is the idea here to support new_estimators = list(getattr(self, attr))should work for other implementations of |
|
@jnothman I don't follow what you mean. Please send me a PR or take over. thx |
|
The problem with assigning the fitted transformer into the The suggestion of @jnothman is also a nice way to fix it (and indeed fixes it for all subclass of BaseComposition), but it has the disadvantage of But maybe in general, do we actually want to allow tuples? (if so, probably need to add a test for it) |
|
I didn't realise this PR was fixing a regression, @agramfort, as #9587 shows this is. |
as pipeline needs to support assignment
Fixes #9587