Remove SimpleImputer DeprecationWarning #1018
Conversation
angela97lin
changed the title
Codecov Report
@@ Coverage Diff @@
## main #1018 +/- ##
==========================================
+ Coverage 99.86% 99.90% +0.04%
==========================================
Files 181 181
Lines 9628 9631 +3
==========================================
+ Hits 9615 9622 +7
+ Misses 13 9 -4
Continue to review full report at Codecov.
|
| @@ -55,10 +52,7 @@ def fit_transform(self, X, y=None): | |||
| else: | |||
| self.input_feature_names = range(X.shape[1]) | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
Removing because these lines are not covered by codecov, and our implementation override of Transformer's fit_transform/transform should mean that we're not checking this here... alternatively I could add in a fake mocked feature selector and add tests?
There was a problem hiding this comment.
This doesn't have anything to do with the SimpleImputer though right?
There was a problem hiding this comment.
@freddyaboulton Nope, but removing the warning lines decreased the total number of lines and hence decreased the overall codecov, causing codecov to fail :(
There was a problem hiding this comment.
@angela97lin is it possible to do this change in a separate PR? Its not required to remove the deprecation warning, right?
There was a problem hiding this comment.
If you needed to remove this in order to get codecov green, I'd say let's either add a couple quick unit tests for these cases, or ignore codecov and handle this separately.
| @@ -55,10 +52,7 @@ def fit_transform(self, X, y=None): | |||
| else: | |||
| self.input_feature_names = range(X.shape[1]) | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
This doesn't have anything to do with the SimpleImputer though right?
| @@ -55,10 +52,7 @@ def fit_transform(self, X, y=None): | |||
| else: | |||
| self.input_feature_names = range(X.shape[1]) | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
@angela97lin is it possible to do this change in a separate PR? Its not required to remove the deprecation warning, right?
| @@ -55,10 +52,7 @@ def fit_transform(self, X, y=None): | |||
| else: | |||
| self.input_feature_names = range(X.shape[1]) | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
If you needed to remove this in order to get codecov green, I'd say let's either add a couple quick unit tests for these cases, or ignore codecov and handle this separately.
Closes #999
After discussion with @dsherry, we decided that it was fine to keep the
SimpleImputerin place, even if we've updated the . ThePerColumnImputercurrently uses theSimpleImputerin its implementation, and updating it to use our newImputerwould make the API more hairy to use. This PR will now simply remove the DeprecationWarning added in v0.12.0.