Skip to content

TST Extend tests for scipy.sparse/*array in sklearn/tests/test_kernel_approximation#27275

Closed
lohitslohit wants to merge 3 commits intoscikit-learn:mainfrom
lohitslohit:spare-array-test-kernel
Closed

TST Extend tests for scipy.sparse/*array in sklearn/tests/test_kernel_approximation#27275
lohitslohit wants to merge 3 commits intoscikit-learn:mainfrom
lohitslohit:spare-array-test-kernel

Conversation

@lohitslohit
Copy link
Copy Markdown
Contributor

Reference Issues/PRs
Towards #27090.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 3, 2023

❌ Linting issues

This PR is introducing linting issues. Here's a summary of the issues. Note that you can avoid having linting issues by enabling pre-commit hooks. Instructions to enable them can be found here.

You can see the details of the linting issues under the lint job here


black

black detected issues. Please run black . locally and push the changes. Here you can see the detected issues. Note that running black might also fix some of the issues which might be detected by ruff. Note that the installed black version is black=23.3.0.

Details

--- /home/runner/work/scikit-learn/scikit-learn/sklearn/tests/test_kernel_approximation.py	2023-09-06 16:12:56.664419 +0000
+++ /home/runner/work/scikit-learn/scikit-learn/sklearn/tests/test_kernel_approximation.py	2023-09-06 16:13:27.748682 +0000
@@ -86,10 +86,11 @@
 
 
 def _linear_kernel(X, Y):
     return np.dot(X, Y.T)
 
+
 @pytest.mark.parametrize("csr_container", CSR_CONTAINERS)
 def test_additive_chi2_sampler():
     # test that AdditiveChi2Sampler approximates kernel on random data
 
     # compute exact kernel
@@ -300,10 +301,11 @@
         skewed_chi2_sampler_32.random_offset_, skewed_chi2_sampler_64.random_offset_
     )
     assert_allclose(
         skewed_chi2_sampler_32.random_weights_, skewed_chi2_sampler_64.random_weights_
     )
+
 
 @pytest.mark.parametrize("csr_container", CSR_CONTAINERS)
 def test_input_validation():
     # Regression test: kernel approx. transformers should work on lists
     # No assertions; the old versions would simply crash
would reformat /home/runner/work/scikit-learn/scikit-learn/sklearn/tests/test_kernel_approximation.py

Oh no! 💥 💔 💥
1 file would be reformatted, 908 files would be left unchanged.

ruff

ruff detected issues. Please run ruff --fix --show-source . locally, fix the remaining issues, and push the changes. Here you can see the detected issues. Note that the installed ruff version is ruff=0.0.287.

Details

sklearn/tests/test_kernel_approximation.py:81:41: F821 Undefined name `csr_container`
   |
79 |         n_components=500, gamma=gamma, degree=degree, coef0=coef0, random_state=42
80 |     )
81 |     Xt_sparse = ps_sparse.fit_transform(csr_container(X))
   |                                         ^^^^^^^^^^^^^ F821
82 |     Yt_sparse = ps_sparse.transform(csr_container(Y))
   |

sklearn/tests/test_kernel_approximation.py:82:37: F821 Undefined name `csr_container`
   |
80 |     )
81 |     Xt_sparse = ps_sparse.fit_transform(csr_container(X))
82 |     Yt_sparse = ps_sparse.transform(csr_container(Y))
   |                                     ^^^^^^^^^^^^^ F821
83 | 
84 |     assert_allclose(Xt_dense, Xt_sparse)
   |

sklearn/tests/test_kernel_approximation.py:114:42: F821 Undefined name `csr_container`
    |
112 |     assert_array_almost_equal(kernel, kernel_approx, 1)
113 | 
114 |     X_sp_trans = transform.fit_transform(csr_container(X))
    |                                          ^^^^^^^^^^^^^ F821
115 |     Y_sp_trans = transform.transform(csr_container(Y))
    |

sklearn/tests/test_kernel_approximation.py:115:38: F821 Undefined name `csr_container`
    |
114 |     X_sp_trans = transform.fit_transform(csr_container(X))
115 |     Y_sp_trans = transform.transform(csr_container(Y))
    |                                      ^^^^^^^^^^^^^ F821
116 | 
117 |     assert_array_equal(X_trans, X_sp_trans.A)
    |

sklearn/tests/test_kernel_approximation.py:315:9: F821 Undefined name `csr_container`
    |
313 |     RBFSampler().fit(X).transform(X)
314 | 
315 |     X = csr_container(X)
    |         ^^^^^^^^^^^^^ F821
316 |     RBFSampler().fit(X).transform(X)
    |

Found 5 errors.

Generated for commit: 55a0a11. Link to the linter CI: here

@Charlie-XIAO
Copy link
Copy Markdown
Contributor

Charlie-XIAO commented Sep 5, 2023

You may first need to read through this. As is also mentioned by the github-actions bot, you will need to enable the pre-commit hooks, so that every time you commit it will run some code style checks.

Also, you need to run pytest for the file you have modified so that you can make sure it works. It is also mentioned in the contributing guide, and in #27090 (comment).

@lohitslohit
Copy link
Copy Markdown
Contributor Author

Thanks @Charlie-XIAO I had completed..

Copy link
Copy Markdown
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @lohitslohit . In case you haven't set up the pre-commit hooks you can also refer to the PR message by the linting bot and see the details to fix the linting issues.

@jjerphan
Copy link
Copy Markdown
Member

jjerphan commented Sep 9, 2023

Closing this PR in preference of #27301.

@jjerphan jjerphan closed this Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants