Skip to content
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

Parameter for stacking missing indicator into iterative imputer #13601

Conversation

DanilBaibak
Copy link
Contributor

Reference Issues/PRs

Continue of the #12583.

What does this implement/fix? Explain your changes.

As was mentioned in #12583, we would be good to add option to stack MissingIndicator transform into the output of the IterativeImputer's transform.

@DanilBaibak
Copy link
Contributor Author

@jnothman @jeremiedbb does it make sense?

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it makes sense :)

One point of difference between simple and iterative however is that there might be sense in including the missing indicators in each iteration of training. But that involves more thinking and I don't think we need to solve it for this to be valuable.

sklearn/tests/test_impute.py Outdated Show resolved Hide resolved
doc/whats_new/v0.21.rst Outdated Show resolved Hide resolved
sklearn/tests/test_impute.py Show resolved Hide resolved
thomasjpfan and others added 3 commits April 13, 2019 14:56
Co-Authored-By: DanilBaibak <danil.baibak@gmail.com>
…mputer' of github.com:DanilBaibak/scikit-learn into Parameter-For-Stacking-MissingIndicator-Into-IterativeImputer
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @DanilBaibak !

sklearn/tests/test_impute.py Show resolved Hide resolved
doc/modules/impute.rst Outdated Show resolved Hide resolved
Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @DanilBaibak

@DanilBaibak
Copy link
Contributor Author

Glad to help 😊

doc/whats_new/v0.21.rst Outdated Show resolved Hide resolved
Co-Authored-By: DanilBaibak <danil.baibak@gmail.com>
@NicolasHug NicolasHug merged commit 50834f7 into scikit-learn:master Apr 15, 2019
@NicolasHug
Copy link
Member

Thanks @DanilBaibak !

jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Apr 25, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants