From f3535cfaf565e7e855daf14f12088efdd9faf0d6 Mon Sep 17 00:00:00 2001 From: ciku Date: Fri, 5 Nov 2021 23:26:57 +0300 Subject: [PATCH 1/4] fix validate parameters in fit for # KernelPCA, --- sklearn/decomposition/_kernel_pca.py | 5 +++-- sklearn/tests/test_common.py | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sklearn/decomposition/_kernel_pca.py b/sklearn/decomposition/_kernel_pca.py index 0efcf2d4fd341..4ebb2a75a3830 100644 --- a/sklearn/decomposition/_kernel_pca.py +++ b/sklearn/decomposition/_kernel_pca.py @@ -257,8 +257,6 @@ def __init__( copy_X=True, n_jobs=None, ): - if fit_inverse_transform and kernel == "precomputed": - raise ValueError("Cannot fit_inverse_transform with a precomputed kernel.") self.n_components = n_components self.kernel = kernel self.kernel_params = kernel_params @@ -434,6 +432,9 @@ def fit(self, X, y=None): K = self._get_kernel(X) self._fit_transform(K) + if self.fit_inverse_transform and self.kernel == "precomputed": + raise ValueError("Cannot fit_inverse_transform with a precomputed kernel.") + if self.fit_inverse_transform: # no need to use the kernel to transform X, use shortcut expression X_transformed = self.eigenvectors_ * np.sqrt(self.eigenvalues_) diff --git a/sklearn/tests/test_common.py b/sklearn/tests/test_common.py index a777e0e716eb1..483c0c7641a63 100644 --- a/sklearn/tests/test_common.py +++ b/sklearn/tests/test_common.py @@ -410,7 +410,6 @@ def test_transformers_get_feature_names_out(transformer): "FeatureUnion", "GridSearchCV", "HalvingGridSearchCV", - "KernelPCA", "Pipeline", "SGDOneClassSVM", "TheilSenRegressor", From 10d0282fa9096e69801dee7384a140621c5e7cbb Mon Sep 17 00:00:00 2001 From: ciku Date: Thu, 11 Nov 2021 20:17:09 +0300 Subject: [PATCH 2/4] fix tests. Add whats new on the current changes --- doc/whats_new/v1.1.rst | 7 +++++++ sklearn/decomposition/_kernel_pca.py | 5 ++--- sklearn/decomposition/tests/test_kernel_pca.py | 5 ++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst index f0d00501d3368..2e92c8eee9fc6 100644 --- a/doc/whats_new/v1.1.rst +++ b/doc/whats_new/v1.1.rst @@ -120,6 +120,7 @@ Changelog A warning will always be raised upon the removal of empty columns. :pr:`21448` by :user:`Oleh Kozynets ` and :user:`Christian Ritter `. + :user:`Christian Ritter . - |Fix| Fix a bug in :class:`linear_model.RidgeClassifierCV` where the method `predict` was performing an `argmax` on the scores obtained from @@ -173,6 +174,12 @@ Changelog instead of `__init__`. :pr:`21434` by :user:`Krum Arnaudov `. + +:mod: `sklearn.decomposition.KernelPCA` +.......................... +-[Fix] :class:`decomposition.KernelPCA` now validates input parameters in `fit` instead of `__init__` + :pr:`21567` by :user: `Maggie Chege ` + :mod:`sklearn.svm` .................. diff --git a/sklearn/decomposition/_kernel_pca.py b/sklearn/decomposition/_kernel_pca.py index 4ebb2a75a3830..bf08c7e215ccc 100644 --- a/sklearn/decomposition/_kernel_pca.py +++ b/sklearn/decomposition/_kernel_pca.py @@ -427,14 +427,13 @@ def fit(self, X, y=None): self : object Returns the instance itself. """ + if self.fit_inverse_transform and self.kernel == "precomputed": + raise ValueError("Cannot fit_inverse_transform with a precomputed kernel.") X = self._validate_data(X, accept_sparse="csr", copy=self.copy_X) self._centerer = KernelCenterer() K = self._get_kernel(X) self._fit_transform(K) - if self.fit_inverse_transform and self.kernel == "precomputed": - raise ValueError("Cannot fit_inverse_transform with a precomputed kernel.") - if self.fit_inverse_transform: # no need to use the kernel to transform X, use shortcut expression X_transformed = self.eigenvectors_ * np.sqrt(self.eigenvalues_) diff --git a/sklearn/decomposition/tests/test_kernel_pca.py b/sklearn/decomposition/tests/test_kernel_pca.py index 72b40ec83e308..27e21175afa7b 100644 --- a/sklearn/decomposition/tests/test_kernel_pca.py +++ b/sklearn/decomposition/tests/test_kernel_pca.py @@ -79,7 +79,10 @@ def test_kernel_pca_invalid_parameters(): ValueError. """ with pytest.raises(ValueError): - KernelPCA(10, fit_inverse_transform=True, kernel="precomputed") + estimator = KernelPCA( + n_components=10, fit_inverse_transform=True, kernel="precomputed" + ) + estimator.fit(np.random.randn(10, 10)) def test_kernel_pca_consistent_transform(): From 68a2e7690f6ba224d83c4fa208b407f91a962c81 Mon Sep 17 00:00:00 2001 From: ciku Date: Thu, 11 Nov 2021 20:33:46 +0300 Subject: [PATCH 3/4] rebase master --- doc/whats_new/v1.1.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst index 2e92c8eee9fc6..7f64a4b54397f 100644 --- a/doc/whats_new/v1.1.rst +++ b/doc/whats_new/v1.1.rst @@ -120,7 +120,6 @@ Changelog A warning will always be raised upon the removal of empty columns. :pr:`21448` by :user:`Oleh Kozynets ` and :user:`Christian Ritter `. - :user:`Christian Ritter . - |Fix| Fix a bug in :class:`linear_model.RidgeClassifierCV` where the method `predict` was performing an `argmax` on the scores obtained from @@ -177,8 +176,9 @@ Changelog :mod: `sklearn.decomposition.KernelPCA` .......................... --[Fix] :class:`decomposition.KernelPCA` now validates input parameters in `fit` instead of `__init__` - :pr:`21567` by :user: `Maggie Chege ` +- [Fix] :class:`decomposition.KernelPCA` now validates input parameters in + `fit` instead of `__init__`. + :pr:`21567` by :user:`Maggie Chege `. :mod:`sklearn.svm` .................. From 588ec25d5b6142b1987eb67ac1f0c6ec6f0ad18f Mon Sep 17 00:00:00 2001 From: ciku Date: Sun, 14 Nov 2021 18:49:43 +0300 Subject: [PATCH 4/4] changes requested --- doc/whats_new/v1.1.rst | 6 +++--- sklearn/decomposition/tests/test_kernel_pca.py | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst index 7f64a4b54397f..57bb1bd36720b 100644 --- a/doc/whats_new/v1.1.rst +++ b/doc/whats_new/v1.1.rst @@ -174,9 +174,9 @@ Changelog :pr:`21434` by :user:`Krum Arnaudov `. -:mod: `sklearn.decomposition.KernelPCA` -.......................... -- [Fix] :class:`decomposition.KernelPCA` now validates input parameters in +:mod:`sklearn.decomposition.KernelPCA` +...................................... +- |Fix| :class:`decomposition.KernelPCA` now validates input parameters in `fit` instead of `__init__`. :pr:`21567` by :user:`Maggie Chege `. diff --git a/sklearn/decomposition/tests/test_kernel_pca.py b/sklearn/decomposition/tests/test_kernel_pca.py index 27e21175afa7b..848b9bb1b9bbf 100644 --- a/sklearn/decomposition/tests/test_kernel_pca.py +++ b/sklearn/decomposition/tests/test_kernel_pca.py @@ -78,10 +78,11 @@ def test_kernel_pca_invalid_parameters(): Tests fitting inverse transform with a precomputed kernel raises a ValueError. """ - with pytest.raises(ValueError): - estimator = KernelPCA( - n_components=10, fit_inverse_transform=True, kernel="precomputed" - ) + estimator = KernelPCA( + n_components=10, fit_inverse_transform=True, kernel="precomputed" + ) + err_ms = "Cannot fit_inverse_transform with a precomputed kernel" + with pytest.raises(ValueError, match=err_ms): estimator.fit(np.random.randn(10, 10))