[MRG+1] Don't use deprecated Imputer in bagging tests#11593
[MRG+1] Don't use deprecated Imputer in bagging tests#11593glemaitre merged 1 commit intoscikit-learn:masterfrom
Conversation
|
fixes #11482 |
|
@amueller merge when CIs are green. |
|
maybe also note the regression? I'm fine with current version though. |
|
@qinhanmin2014 which regression / where? |
|
Not sure but you might refer to e.g., |
|
@qinhanmin2014 The test is unrelated to imputation so adding anything about changes in the imputation api here seems pretty cryptic / out of place. |
|
My point is that to close #11482 , it might be better to note down situations we no longer support. |
SimpeImputer is a new class so I would not bother. And supporting imputation with np.inf seems wrong. |
|
|
||
|
|
||
| def replace(X): | ||
| X = X.copy().astype('float') |
There was a problem hiding this comment.
Sorry to necro this, but astype copies by default. Should there still be a copy call here? Admittedly I know nothing about this code. So feel free to correct me if I'm missing something.
There was a problem hiding this comment.
you're right. This is a hack used in a test, though, so I don't think it matters. We could remove the copy() though.
Should be in the spirit of the test. I don't think passing inf as missing values makes sense and SimpleImputer doesn't support it.