Adding the median option to the KNN imputer. #27890

blueprintparadise · Dec 2, 2023

Current version only supports mean but there seems to be a median version showing good results. So , I added it to the repo using the "strategy" argument

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Imputer could only have a mean option in the earlier version.

Any other comments?

Apologies if I made some error in the pull request, I am still learning.
Please let me know if there is anything I missed or needs to be changed.

Current version only supports mean but there seems to be a median version showing good results. So , I added it to the repo using the "strategy" argument

github-actions · Dec 2, 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.


--- /home/runner/work/scikit-learn/scikit-learn/sklearn/impute/_knn.py	2023-12-04 12:41:42.565797 +0000
+++ /home/runner/work/scikit-learn/scikit-learn/sklearn/impute/_knn.py	2023-12-04 12:41:59.939548 +0000
@@ -208,11 +208,11 @@
 
         if self.strategy == "mean":
             imputed_values = np.ma.average(donors, axis=1, weights=weight_matrix).data
         elif self.strategy == "median":
             imputed_values = np.ma.median(donors, axis=1).data
-            
+
         return imputed_values
 
     @_fit_context(prefer_skip_nested_validation=True)
     def fit(self, X, y=None):
         """Fit the imputer on X.
would reformat /home/runner/work/scikit-learn/scikit-learn/sklearn/impute/_knn.py

Oh no! 💥 💔 💥
1 file would be reformatted, 907 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.1.6.


sklearn/impute/_knn.py:213:1: W293 [*] Blank line contains whitespace
    |
211 |         elif self.strategy == "median":
212 |             imputed_values = np.ma.median(donors, axis=1).data
213 |             
    | ^^^^^^^^^^^^ W293
214 |         return imputed_values
    |
    = help: Remove whitespace from blank line

Found 1 error.
[*] 1 fixable with the `--fix` option.

_{Generated for commit: 99eb9f5. Link to the linter CI: here}

minor update

removed blank space

remove more blank spaces

solving codecov error

glemaitre · Dec 5, 2023

The CIs are failing. We also need unit tests to check the implementation works as expected.
Do you have any scientific literature supporting this strategy?

Adding the median option to the KNN imputer.

1b3bd8a

Current version only supports mean but there seems to be a median version showing good results. So , I added it to the repo using the "strategy" argument

github-actions bot added the module:impute label Dec 2, 2023

blueprintparadise added 6 commits December 3, 2023 00:09

Update _knn.py

04d658d

minor update

Update _knn.py

b068d1c

removed blank space

Update _knn.py

d3459af

remove more blank spaces

Merge branch 'main' into main

0a2ab6f

Update _knn.py

e401603

solving codecov error

Merge branch 'main' of https://github.com/blueprintparadise/scikit-learn

99eb9f5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Adding the median option to the KNN imputer. #27890

Adding the median option to the KNN imputer. #27890

Uh oh!

blueprintparadise commented Dec 2, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Dec 2, 2023 •

edited

Loading

Uh oh!

glemaitre commented Dec 5, 2023

Uh oh!

Uh oh!

Search code, repositories, users, issues, pull requests...

Uh oh!

Adding the median option to the KNN imputer. #27890

Are you sure you want to change the base?

Adding the median option to the KNN imputer. #27890

Uh oh!

Conversation

blueprintparadise commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

github-actions bot commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Linting issues

black

ruff

Uh oh!

glemaitre commented Dec 5, 2023

Uh oh!

Uh oh!

blueprintparadise commented Dec 2, 2023 •

edited

Loading

github-actions bot commented Dec 2, 2023 •

edited

Loading

`black`

`ruff`