From fae0ef705df2915a704cd1e46e79789395dc9e0f Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 9 Dec 2019 15:49:46 +0100 Subject: [PATCH 1/9] Do not shadow public functions with deprecated modules Fix #15842. This can happen for: - sklearn.decomposition.dict_learning - sklearn.inspection.partial_dependence --- sklearn/decomposition/__init__.py | 33 ++++++++++++------ sklearn/inspection/__init__.py | 20 ++++++++--- sklearn/utils/tests/test_deprecated_utils.py | 35 ++++++++++++++++++++ 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/sklearn/decomposition/__init__.py b/sklearn/decomposition/__init__.py index 93a51b04f38d2..76eb3dbafe575 100644 --- a/sklearn/decomposition/__init__.py +++ b/sklearn/decomposition/__init__.py @@ -4,19 +4,30 @@ this module can be regarded as dimensionality reduction techniques. """ -from ._nmf import NMF, non_negative_factorization -from ._pca import PCA -from ._incremental_pca import IncrementalPCA -from ._kernel_pca import KernelPCA -from ._sparse_pca import SparsePCA, MiniBatchSparsePCA -from ._truncated_svd import TruncatedSVD -from ._fastica import FastICA, fastica +# TODO: remove me in 0.24: +# pre-cache the import of the deprecated module to avoid showing +# the partial_dependence function. +# https://github.com/scikit-learn/scikit-learn/issues/15842 +import warnings +with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=FutureWarning) + from . import dict_learning as _ignored + del _ignored + + +from ._nmf import NMF, non_negative_factorization # noqa +from ._pca import PCA # noqa +from ._incremental_pca import IncrementalPCA # noqa +from ._kernel_pca import KernelPCA # noqa +from ._sparse_pca import SparsePCA, MiniBatchSparsePCA # noqa +from ._truncated_svd import TruncatedSVD # noqa +from ._fastica import FastICA, fastica # noqa from ._dict_learning import (dict_learning, dict_learning_online, sparse_encode, DictionaryLearning, - MiniBatchDictionaryLearning, SparseCoder) -from ._factor_analysis import FactorAnalysis -from ..utils.extmath import randomized_svd -from ._online_lda import LatentDirichletAllocation + MiniBatchDictionaryLearning, SparseCoder) # noqa +from ._factor_analysis import FactorAnalysis # noqa +from ..utils.extmath import randomized_svd # noqa +from ._online_lda import LatentDirichletAllocation # noqa __all__ = ['DictionaryLearning', 'FastICA', diff --git a/sklearn/inspection/__init__.py b/sklearn/inspection/__init__.py index 04d9d84ecaf02..7db8ce08c6345 100644 --- a/sklearn/inspection/__init__.py +++ b/sklearn/inspection/__init__.py @@ -1,8 +1,20 @@ """The :mod:`sklearn.inspection` module includes tools for model inspection.""" -from ._partial_dependence import partial_dependence -from ._partial_dependence import plot_partial_dependence -from ._partial_dependence import PartialDependenceDisplay -from ._permutation_importance import permutation_importance + +# TODO: remove me in 0.24: +# pre-cache the import of the deprecated module to avoid showing +# the partial_dependence function. +# https://github.com/scikit-learn/scikit-learn/issues/15842 +import warnings +with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=FutureWarning) + from . import partial_dependence as _ignored + del _ignored + +from ._partial_dependence import partial_dependence # noqa +from ._partial_dependence import plot_partial_dependence # noqa +from ._partial_dependence import PartialDependenceDisplay # noqa +from ._permutation_importance import permutation_importance # noqa + __all__ = [ 'partial_dependence', diff --git a/sklearn/utils/tests/test_deprecated_utils.py b/sklearn/utils/tests/test_deprecated_utils.py index da41e7e44ddb3..c69f8249e138c 100644 --- a/sklearn/utils/tests/test_deprecated_utils.py +++ b/sklearn/utils/tests/test_deprecated_utils.py @@ -1,7 +1,10 @@ import pytest +import types import numpy as np +import warnings from sklearn.dummy import DummyClassifier +from sklearn.utils import all_estimators from sklearn.utils.estimator_checks import choose_check_classifiers_labels from sklearn.utils.estimator_checks import NotAnArray from sklearn.utils.estimator_checks import enforce_estimator_tags_y @@ -94,3 +97,35 @@ def test_safe_indexing(): with pytest.warns(FutureWarning, match="removed in version 0.24"): safe_indexing([1, 2], 0) + + +# TODO: remove in 0.24 +def test_partial_dependence_no_shadowing(): + # Non-regression test for: + # https://github.com/scikit-learn/scikit-learn/issues/15842 + with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=FutureWarning) + from sklearn.inspection.partial_dependence import partial_dependence as _ # noqa + + # Calling all_estimators() also triggers a recursive import of all + # submodules, including deprecated once. + all_estimators() + + from sklearn.inspection import partial_dependence + assert isinstance(partial_dependence, types.FunctionType) + + +# TODO: remove in 0.24 +def test_dict_learning_no_shadowing(): + # Non-regression test for: + # https://github.com/scikit-learn/scikit-learn/issues/15842 + with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=FutureWarning) + from sklearn.decomposition.dict_learning import dict_learning as _ # noqa + + # Calling all_estimators() also triggers a recursive import of all + # submodules, including deprecated once. + all_estimators() + + from sklearn.decomposition import dict_learning + assert isinstance(dict_learning, types.FunctionType) From 31614fe0e797b9486d2d8901d9f5b55a9a0ab817 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 9 Dec 2019 16:29:39 +0100 Subject: [PATCH 2/9] Special case in test_import_is_deprecated --- sklearn/tests/test_import_deprecations.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sklearn/tests/test_import_deprecations.py b/sklearn/tests/test_import_deprecations.py index 13b31e89b2862..29c4259fe1e5a 100644 --- a/sklearn/tests/test_import_deprecations.py +++ b/sklearn/tests/test_import_deprecations.py @@ -24,6 +24,12 @@ def test_import_is_deprecated(deprecated_path, importee): # TODO: remove in 0.24 + # Special case for: + # https://github.com/scikit-learn/scikit-learn/issues/15842 + if deprecated_path in ("sklearn.decomposition.dict_learning", + "sklearn.inspection.partial_dependence"): + pytest.skip("No warning can be raised for " + deprecated_path) + expected_message = ( "The {deprecated_path} module is deprecated in version " "0.22 and will be removed in version 0.24. " From 2dd072aeb24217d1891e97d747d6f31667c2c322 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 9 Dec 2019 16:39:54 +0100 Subject: [PATCH 3/9] typo [ci skip] --- sklearn/utils/tests/test_deprecated_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sklearn/utils/tests/test_deprecated_utils.py b/sklearn/utils/tests/test_deprecated_utils.py index c69f8249e138c..08bd95aacc284 100644 --- a/sklearn/utils/tests/test_deprecated_utils.py +++ b/sklearn/utils/tests/test_deprecated_utils.py @@ -108,7 +108,7 @@ def test_partial_dependence_no_shadowing(): from sklearn.inspection.partial_dependence import partial_dependence as _ # noqa # Calling all_estimators() also triggers a recursive import of all - # submodules, including deprecated once. + # submodules, including deprecated ones. all_estimators() from sklearn.inspection import partial_dependence @@ -124,7 +124,7 @@ def test_dict_learning_no_shadowing(): from sklearn.decomposition.dict_learning import dict_learning as _ # noqa # Calling all_estimators() also triggers a recursive import of all - # submodules, including deprecated once. + # submodules, including deprecated ones. all_estimators() from sklearn.decomposition import dict_learning From 2557bf5707f37f1ec2cfb26e5abb0abfbbf5d379 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 9 Dec 2019 16:41:24 +0100 Subject: [PATCH 4/9] trigger ci (pushed previous commit too quickly) From f246839ea246864fb6b862bcf5bdc23240d0bb91 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 9 Dec 2019 21:46:31 +0100 Subject: [PATCH 5/9] Update sklearn/decomposition/__init__.py Co-Authored-By: Nicolas Hug --- sklearn/decomposition/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/decomposition/__init__.py b/sklearn/decomposition/__init__.py index 76eb3dbafe575..c792034982c2b 100644 --- a/sklearn/decomposition/__init__.py +++ b/sklearn/decomposition/__init__.py @@ -4,7 +4,7 @@ this module can be regarded as dimensionality reduction techniques. """ -# TODO: remove me in 0.24: +# TODO: remove me in 0.24, as well as the #noqas: # pre-cache the import of the deprecated module to avoid showing # the partial_dependence function. # https://github.com/scikit-learn/scikit-learn/issues/15842 From 83e35b38f0c164f36e66e94b252a3d9fc32db930 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 9 Dec 2019 21:50:48 +0100 Subject: [PATCH 6/9] Improve phrasing --- sklearn/decomposition/__init__.py | 7 ++++--- sklearn/inspection/__init__.py | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sklearn/decomposition/__init__.py b/sklearn/decomposition/__init__.py index c792034982c2b..6646382dc1ef8 100644 --- a/sklearn/decomposition/__init__.py +++ b/sklearn/decomposition/__init__.py @@ -4,9 +4,10 @@ this module can be regarded as dimensionality reduction techniques. """ -# TODO: remove me in 0.24, as well as the #noqas: -# pre-cache the import of the deprecated module to avoid showing -# the partial_dependence function. +# TODO: remove me in 0.24 (as well as the noqa markers) +# pre-cache the import of the deprecated module so that import +# sklearn.decomposition.dict_learning returns the function as in +# 0.21, instead of the module. # https://github.com/scikit-learn/scikit-learn/issues/15842 import warnings with warnings.catch_warnings(): diff --git a/sklearn/inspection/__init__.py b/sklearn/inspection/__init__.py index 7db8ce08c6345..4a3f00006ac10 100644 --- a/sklearn/inspection/__init__.py +++ b/sklearn/inspection/__init__.py @@ -1,8 +1,9 @@ """The :mod:`sklearn.inspection` module includes tools for model inspection.""" -# TODO: remove me in 0.24: -# pre-cache the import of the deprecated module to avoid showing -# the partial_dependence function. +# TODO: remove me in 0.24 (as well as the noqa markers) +# pre-cache the import of the deprecated module so that import +# sklearn.inspection.partial_dependence returns the function as in +# 0.21, instead of the module # https://github.com/scikit-learn/scikit-learn/issues/15842 import warnings with warnings.catch_warnings(): From fe82e78df8df05a39fc36b41c12bf31f35054495 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Mon, 16 Dec 2019 11:15:43 +0100 Subject: [PATCH 7/9] Use the 0.21 imports --- sklearn/decomposition/__init__.py | 11 ++++++----- sklearn/inspection/__init__.py | 10 +++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/sklearn/decomposition/__init__.py b/sklearn/decomposition/__init__.py index 6646382dc1ef8..419176ab33194 100644 --- a/sklearn/decomposition/__init__.py +++ b/sklearn/decomposition/__init__.py @@ -4,16 +4,17 @@ this module can be regarded as dimensionality reduction techniques. """ -# TODO: remove me in 0.24 (as well as the noqa markers) -# pre-cache the import of the deprecated module so that import +# TODO: remove me in 0.24 (as well as the noqa markers) and +# import the dict_learning func directly from the ._dict_learning +# module instead. +# Pre-cache the import of the deprecated module so that import # sklearn.decomposition.dict_learning returns the function as in # 0.21, instead of the module. # https://github.com/scikit-learn/scikit-learn/issues/15842 import warnings with warnings.catch_warnings(): warnings.simplefilter("ignore", category=FutureWarning) - from . import dict_learning as _ignored - del _ignored + from .dict_learning import dict_learning from ._nmf import NMF, non_negative_factorization # noqa @@ -23,7 +24,7 @@ from ._sparse_pca import SparsePCA, MiniBatchSparsePCA # noqa from ._truncated_svd import TruncatedSVD # noqa from ._fastica import FastICA, fastica # noqa -from ._dict_learning import (dict_learning, dict_learning_online, +from ._dict_learning import (dict_learning_online, sparse_encode, DictionaryLearning, MiniBatchDictionaryLearning, SparseCoder) # noqa from ._factor_analysis import FactorAnalysis # noqa diff --git a/sklearn/inspection/__init__.py b/sklearn/inspection/__init__.py index 4a3f00006ac10..904d16d74b016 100644 --- a/sklearn/inspection/__init__.py +++ b/sklearn/inspection/__init__.py @@ -1,17 +1,17 @@ """The :mod:`sklearn.inspection` module includes tools for model inspection.""" -# TODO: remove me in 0.24 (as well as the noqa markers) -# pre-cache the import of the deprecated module so that import +# TODO: remove me in 0.24 (as well as the noqa markers) and +# import the partial_dependence func directly from the +# ._partial_dependence module instead. +# Pre-cache the import of the deprecated module so that import # sklearn.inspection.partial_dependence returns the function as in # 0.21, instead of the module # https://github.com/scikit-learn/scikit-learn/issues/15842 import warnings with warnings.catch_warnings(): warnings.simplefilter("ignore", category=FutureWarning) - from . import partial_dependence as _ignored - del _ignored + from .partial_dependence import partial_dependence -from ._partial_dependence import partial_dependence # noqa from ._partial_dependence import plot_partial_dependence # noqa from ._partial_dependence import PartialDependenceDisplay # noqa from ._permutation_importance import permutation_importance # noqa From be9070ae83049ad7741c4dc01abf7fe7a1f9aa57 Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Tue, 17 Dec 2019 11:57:25 +0100 Subject: [PATCH 8/9] Trigger CI From eee29852647b03d1e3fc1920f779f34cf400cd1b Mon Sep 17 00:00:00 2001 From: Olivier Grisel Date: Thu, 19 Dec 2019 17:26:40 +0100 Subject: [PATCH 9/9] Trigger CI