[MRG+1] Deprecate axis parameter in imputer#10558
[MRG+1] Deprecate axis parameter in imputer#10558jnothman merged 10 commits intoscikit-learn:masterfrom qinhanmin2014:deprecate_imputer_axis
Conversation
…and equals to 0 when axis is None.
|
LGTM |
|
@jnothman Since that you reviewed the original PR, I think that this is ready to be merged. |
jnothman
left a comment
There was a problem hiding this comment.
doc/modules/preprocessing.rst sets axis explicitly and will now raise a warning. examples/plot_missing_values.py does too. They should be changed.
sklearn/preprocessing/imputation.py
Outdated
| warnings.warn("Parameter 'axis' has been deprecated in 0.20 and " | ||
| "will be removed in 0.22. Future (and default) " | ||
| "behavior is equivalent to 'axis=0' (impute along " | ||
| "columns).", DeprecationWarning) |
There was a problem hiding this comment.
We should probably say "Row-wise imputation can be performed with FunctionTransformer." Could even specify FunctionTransformer(lambda X: Imputer(...).fit_transform(X.T).T)
|
Thanks @glemaitre @jnothman :) |
|
FYI, this seems to have broken a test: The issue is that the axis check was not altered to be I can fix this in the MICE PR, as this change also effects how MICE will make use of |
|
The fix is here: 8350981 |
|
Please submit a separate PR... why is this not failing in master? |
|
OK will do. No idea why it's not failing in master. |
|
Hmm, it looks correct in master. It's probably a mistake I made while merging the MICE PR to master, but I'm not sure how that would have happened. My apologies for the bother! |
Reference Issues/PRs
Fixes #9463
Closes #9672
What does this implement/fix? Explain your changes.
We are unable to contact with the author of the original PR, so I try to complete it.
Improvements (according to the reviews in #9672):
(1) Improve the deprecation message
(2) Add a test
(3) Correct what's new
(4) Ignore deprecation warnings in the tests (since there are too many warnings)
Any other comments?