Repository: scikit-learn/scikit-learn
Pull request: ENH Improves error message when experimental flag is not imported
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/main/CONTRIBUTING.md
-->

#### Reference Issues/PRs
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->
Fixes https://github.com/scikit-learn/scikit-learn/issues/13964

#### What does this implement/fix? Explain your changes.
This PR uses PEP 562 to display a better error message when `IterativeImputer` , `HalvingRandomSearchCV`, or `HalvingGridSearchCV` is imported without the experimental import flag


<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
Experimental error message 
```py
# from sklearn.experimental import enable_hist_gradient_boosting
from sklearn.ensemble import HistGradientBoostingRegressor
```
gives 

```sh
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-472ba833df30> in <module>
      1 # from sklearn.experimental import enable_hist_gradient_boosting
----> 2 from sklearn.ensemble import HistGradientBoostingRegressor

ImportError: cannot import name 'HistGradientBoostingRegressor' from 'sklearn.ensemble' (/srv/conda/envs/notebook/lib/python3.7/site-packages/sklearn/ensemble/__init__.py)
```

It would be good if the import error points out how to enable the import.

Can we introspect it somehow to improve the error message? has this been discussed somewhere?

Hints from issue discussion:
It would be quite hard to make this foolproof. You'd need an import hook
and something to handle attribute access. I don't think it's warranted.

Do you think it is worth the effort? otherwise, feel free to close.
Yeah that's one of the main downsides of this solution.

As Joel said, it's very hard to have a custom error message. Unless we bump our requirements to 3.7... ([PEP 562](https://www.python.org/dev/peps/pep-0562/))
Hmm... I suppose solving this in python>=3.7 would be good and wouldn't
break other versions.

I am +1 on a python>=3.7 solution that would **not break** older versions.
You meant would *not* break @thomasjpfan ?
Yes I meant not break.

Commit message: CLN Move all experimental import logic into __getattr__

Patch to review:
diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst
index 840abe0a45358..3f3431c8688aa 100644
--- a/doc/whats_new/v1.1.rst
+++ b/doc/whats_new/v1.1.rst
@@ -150,6 +150,12 @@ Changelog
   error message when setting invalid hyper-parameters with `set_params`.
   :pr:`21542` by :user:`Olivier Grisel <ogrisel>`.
 
+- |Enhancement| The error message is improved when importing
+  :class:`model_selection.HalvingGridSearchCV`,
+  :class:`model_selection.HalvingRandomSearchCV`, or
+  :class:`impute.IterativeImputer` without importing the experimental flag.
+  :pr:`23194` by `Thomas Fan`_.
+
 :mod:`sklearn.calibration`
 ..........................
 
@@ -297,7 +303,7 @@ Changelog
 - |MajorFeature| Added a new estimator :class:`decomposition.MiniBatchNMF`. It is a
   faster but less accurate version of non-negative matrix factorization, better suited
   for large datasets. :pr:`16948` by :user:`Chiara Marmo <cmarmo>`,
-  :user:`Patricio Cerda <pcerda>` and :user:`Jérémie du Boisberranger <jeremiedbb>`. 
+  :user:`Patricio Cerda <pcerda>` and :user:`Jérémie du Boisberranger <jeremiedbb>`.
 
 - |Enhancement| :class:`decomposition.PCA` exposes a parameter `n_oversamples` to tune
   :func:`sklearn.decomposition.randomized_svd` and
diff --git a/sklearn/experimental/enable_halving_search_cv.py b/sklearn/experimental/enable_halving_search_cv.py
index f6937b0d14c01..6fa231cb0afe6 100644
--- a/sklearn/experimental/enable_halving_search_cv.py
+++ b/sklearn/experimental/enable_halving_search_cv.py
@@ -18,16 +18,3 @@
 The ``# noqa`` comment comment can be removed: it just tells linters like
 flake8 to ignore the import, which appears as unused.
 """
-
-from ..model_selection._search_successive_halving import (
-    HalvingRandomSearchCV,
-    HalvingGridSearchCV,
-)
-
-from .. import model_selection
-
-# use settattr to avoid mypy errors when monkeypatching
-setattr(model_selection, "HalvingRandomSearchCV", HalvingRandomSearchCV)
-setattr(model_selection, "HalvingGridSearchCV", HalvingGridSearchCV)
-
-model_selection.__all__ += ["HalvingRandomSearchCV", "HalvingGridSearchCV"]
diff --git a/sklearn/experimental/enable_iterative_imputer.py b/sklearn/experimental/enable_iterative_imputer.py
index 9ef9f6a0dbdf0..6ca194917ef48 100644
--- a/sklearn/experimental/enable_iterative_imputer.py
+++ b/sklearn/experimental/enable_iterative_imputer.py
@@ -11,10 +11,3 @@
     >>> # now you can import normally from impute
     >>> from sklearn.impute import IterativeImputer
 """
-
-from ..impute._iterative import IterativeImputer
-from .. import impute
-
-# use settattr to avoid mypy errors when monkeypatching
-setattr(impute, "IterativeImputer", IterativeImputer)
-impute.__all__ += ["IterativeImputer"]
diff --git a/sklearn/experimental/tests/test_enable_iterative_imputer.py b/sklearn/experimental/tests/test_enable_iterative_imputer.py
index 21d57b37c519c..3f4ce37f7afcc 100644
--- a/sklearn/experimental/tests/test_enable_iterative_imputer.py
+++ b/sklearn/experimental/tests/test_enable_iterative_imputer.py
@@ -29,11 +29,11 @@ def test_imports_strategies():
     bad_imports = """
     import pytest
 
-    with pytest.raises(ImportError):
+    with pytest.raises(ImportError, match='IterativeImputer is experimental'):
         from sklearn.impute import IterativeImputer
 
     import sklearn.experimental
-    with pytest.raises(ImportError):
+    with pytest.raises(ImportError, match='IterativeImputer is experimental'):
         from sklearn.impute import IterativeImputer
     """
     assert_run_python_script(textwrap.dedent(bad_imports))
diff --git a/sklearn/experimental/tests/test_enable_successive_halving.py b/sklearn/experimental/tests/test_enable_successive_halving.py
index 04435e690934f..4aa695e654ccc 100644
--- a/sklearn/experimental/tests/test_enable_successive_halving.py
+++ b/sklearn/experimental/tests/test_enable_successive_halving.py
@@ -31,11 +31,11 @@ def test_imports_strategies():
     bad_imports = """
     import pytest
 
-    with pytest.raises(ImportError):
+    with pytest.raises(ImportError, match='HalvingGridSearchCV is experimental'):
         from sklearn.model_selection import HalvingGridSearchCV
 
     import sklearn.experimental
-    with pytest.raises(ImportError):
-        from sklearn.model_selection import HalvingGridSearchCV
+    with pytest.raises(ImportError, match='HalvingRandomSearchCV is experimental'):
+        from sklearn.model_selection import HalvingRandomSearchCV
     """
     assert_run_python_script(textwrap.dedent(bad_imports))
diff --git a/sklearn/impute/__init__.py b/sklearn/impute/__init__.py
index 48cf8acae9be4..dcbffcfc0218d 100644
--- a/sklearn/impute/__init__.py
+++ b/sklearn/impute/__init__.py
@@ -1,5 +1,6 @@
 """Transformers for missing value imputation"""
 import typing
+import sys
 
 from ._base import MissingIndicator, SimpleImputer
 from ._knn import KNNImputer
@@ -10,3 +11,19 @@
     from ._iterative import IterativeImputer  # noqa
 
 __all__ = ["MissingIndicator", "SimpleImputer", "KNNImputer"]
+
+
+# TODO: remove this check once the estimator is no longer experimental.
+def __getattr__(name):
+    if name == "IterativeImputer":
+        if "sklearn.experimental.enable_iterative_imputer" not in sys.modules:
+            raise ImportError(
+                f"{name} is experimental and the API might change without any "
+                "deprecation cycle. To use it, you need to explicitly import "
+                "enable_iterative_imputer:\n"
+                "from sklearn.experimental import enable_iterative_imputer"
+            )
+        from . import _iterative
+
+        return getattr(_iterative, name)
+    raise AttributeError(f"module {__name__} has no attribute {name}")
diff --git a/sklearn/model_selection/__init__.py b/sklearn/model_selection/__init__.py
index 580bb778e9ece..0b1e4e93151e7 100644
--- a/sklearn/model_selection/__init__.py
+++ b/sklearn/model_selection/__init__.py
@@ -1,4 +1,5 @@
 import typing
+import sys
 
 from ._split import BaseCrossValidator
 from ._split import BaseShuffleSplit
@@ -72,3 +73,19 @@
     "train_test_split",
     "validation_curve",
 ]
+
+
+# TODO: remove this check once the estimator is no longer experimental.
+def __getattr__(name):
+    if name in {"HalvingGridSearchCV", "HalvingRandomSearchCV"}:
+        if "sklearn.experimental.enable_halving_search_cv" not in sys.modules:
+            raise ImportError(
+                f"{name} is experimental and the API might change without any "
+                "deprecation cycle. To use it, you need to explicitly import "
+                "enable_halving_search_cv:\n"
+                "from sklearn.experimental import enable_halving_search_cv"
+            )
+        from . import _search_successive_halving
+
+        return getattr(_search_successive_halving, name)
+    raise AttributeError(f"module {__name__} has no attribute {name}")


Code review comments:
diff_hunk: @@ -18,16 +18,3 @@
 The ``# noqa`` comment comment can be removed: it just tells linters like
 flake8 to ignore the import, which appears as unused.
 """
-
-from ..model_selection._search_successive_halving import (
-    HalvingRandomSearchCV,
-    HalvingGridSearchCV,
-)
-
-from .. import model_selection
-
-# use settattr to avoid mypy errors when monkeypatching
-setattr(model_selection, "HalvingRandomSearchCV", HalvingRandomSearchCV)
-setattr(model_selection, "HalvingGridSearchCV", HalvingGridSearchCV)
-
-model_selection.__all__ += ["HalvingRandomSearchCV", "HalvingGridSearchCV"]
path: sklearn/experimental/enable_halving_search_cv.py
text: @user1:
The `__all__` functionality has not been replicated in `__getattr__`, is that on purpose?

@author:
I have not consider `__all__`. I do not think there is a way to replicate it in `__getattr__`. [PEP562](https://peps.python.org/pep-0562/) allows us to adjust `dir`, but that does not influence `__all__` or the `import *` mechanism.

With that in mind, I reverted the commit that moves everything into `__getattr__`.