-
-
Notifications
You must be signed in to change notification settings - Fork 26.2k
[MRG] Initialize MissingIndicator with error_on_new = False #13974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Please add a non-regression test. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presume this also applies to IterativeImputer
Would the test be to ensure that errors are thrown if relevant features are missing values? |
To test that no errors are raised if a feature has a missing value in test
but not in train, and that the transformed data is all finite.
|
…e IterativeImputer nonregression test and also default error_on_new to False.
@jnothman can you review? Thanks! |
Please revert the changes that are not related to the PR. |
Done! @thomasjpfan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an entry to the change log at doc/whats_new/v0.21.rst
under the 0.21.3 change log heading. Like the other entries there, please reference this pull request with :pr:
and credit yourself (and other contributors if applicable) with :user:
Done! @jnothman. Thanks for the help/input. |
@thomasjpfan: change being pushed up. |
@NicolasHug I proposed putting this is an 0.21 series bug fix release, assuming we have a couple more fixes to join if (not sure if you want the recently raised divide by zero in gradient boosting to be fixed in 0.21.3 also, for instance) |
Agree this should be a bug-fix release. |
Implemented suggestions @NicolasHug @amueller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM.
Thank you @fhoang7! |
Reference Issues/PRs
Resolves #13968
What does this implement/fix? Explain your changes.
Set error_on_new to False by default if add_indicator is True in order to suppress missing value errors in ignored features.
Any other comments?