-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
API Make IterativeImputer experimental #13824
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
You'll also need to import |
I take it from
#13789 (comment)
that @glemaitre notionally approves. Anyone else?
|
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.
IRL @ogrisel was only +1. I am on the phone and don't want to merge from there. But it should be enough approval to go ahead.
@@ -105,7 +105,16 @@ of ``y``. This is done for each feature in an iterative fashion, and then is | |||
repeated for ``max_iter`` imputation rounds. The results of the final | |||
imputation round are returned. | |||
|
|||
.. note:: |
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.
Could you make a small change in l. 25 and 27 of this file. The hyperlinks does not work:
- :class:
impute.SimpleImputer
-> :class:SimpleImputer
- :class:
impute.IterativeImputer
-> :class:IterativeImputer
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.
LGTM except for some copy-paste left-overs.
sklearn/impute/_iterative.py
Outdated
|
||
This estimator is still **experimental** for now: the predictions | ||
and the API might change without any deprecation cycle. To use it, | ||
you need to explicitly import ``enable_hist_gradient_boosting``:: |
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.
enable_iterative_imputer
sklearn/impute/_iterative.py
Outdated
|
||
>>> # explicitly require this experimental feature | ||
>>> from sklearn.experimental import enable_iterative_imputer # noqa | ||
>>> # now you can import normally from ensemble |
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.
from impute
|
||
>>> # explicitly require this experimental feature | ||
>>> from sklearn.experimental import enable_iterative_imputer # noqa | ||
>>> # now you can import normally from ensemble |
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.
from impute
Thanks for finding those. I did this in a rush
|
Sorry for the merge conflicts on KNNImputer, @thomasjpfan! |
# discovered properly by sphinx | ||
from sklearn.experimental import enable_hist_gradient_boosting # noqa |
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.
@jnothman I believe this does not work as expected:
>>> from sklearn.experimental import *
>>> from sklearn.ensemble import HistGradientBoostingRegressor
Traceback (most recent call last):
File "<ipython-input-5-47b8aeb492fe>", line 1, in <module>
from sklearn.ensemble import HistGradientBoostingRegressor
ImportError: cannot import name 'HistGradientBoostingRegressor' from 'sklearn.ensemble' (/home/ogrisel/code/scikit-learn/sklearn/ensemble/__init__.py)
>>> from sklearn.experimental import enable_hist_gradient_boosting
>>> from sklearn.ensemble import HistGradientBoostingRegressor
>>>
Fixes #13789 .
See #13773. The point is that we've made some design and algorithmic choices that may deserve refinement over the next release or two without requiring backwards compatibility.
We need to either accept or reject this change before release of 0.21.0