Skip to content

DOC improve the KNN classifier example#26993

Merged
adrinjalali merged 5 commits intoscikit-learn:mainfrom
glemaitre:knn_classification_example
Aug 3, 2023
Merged

DOC improve the KNN classifier example#26993
adrinjalali merged 5 commits intoscikit-learn:mainfrom
glemaitre:knn_classification_example

Conversation

@glemaitre
Copy link
Copy Markdown
Member

Improving slightly the KNN classifier example.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 2, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: d394000. Link to the linter CI: here

Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

This definitely looks much better 😁

glemaitre and others added 2 commits August 2, 2023 15:18
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Copy link
Copy Markdown
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Just a couple of small comments, otherwise LGTM. Thanks @glemaitre :)

# Conclusion
# ----------
#
# We observe that the parameter `weights` has an impact on the decision boundary. When
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems to be a very slight impact. I don't know if we can do something about it, though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nop. I would expect to have a greater impact when you have neighbours that are actually far away but I don't think this is useful to build a toy example for that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It might be useful in higher-dimension. Not sure.

glemaitre and others added 2 commits August 3, 2023 12:44
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
@adrinjalali adrinjalali merged commit 38a06e4 into scikit-learn:main Aug 3, 2023
TamaraAtanasoska pushed a commit to TamaraAtanasoska/scikit-learn that referenced this pull request Aug 21, 2023
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
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.

3 participants