Skip to content

MNT Fix error message for Minkowski metric parameter - #19671

Merged
rth merged 3 commits into
scikit-learn:mainfrom
zouharvi:patch-1
Mar 15, 2021
Merged

rth merged 3 commits into
scikit-learn:mainfrom
zouharvi:patch-1

Conversation

@zouharvi

Copy link
Copy Markdown
Contributor

It is possible to pass p=1 for minkowski metric, even though internally the effective metric gets changed to manhattan.

This PR simply edits the two error messages to not confuse the users.

It is possible to pass p=1 for `minkowski` metric, even though internally the effective metric gets changed to `manhattan`. This PR simply edits the two error messages to not confuse the users.

@thomasjpfan thomasjpfan left a comment

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.

Thank you for the PR @zouharvi !

Comment thread sklearn/neighbors/_base.py Outdated
Comment thread sklearn/neighbors/_base.py Outdated
zouharvi and others added 2 commits March 13, 2021 19:04
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

@thomasjpfan thomasjpfan left a comment

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.

LGTM

@thomasjpfan thomasjpfan changed the title Fix error message for Minkowski metric parameter MNT Fix error message for Minkowski metric parameter Mar 13, 2021

@rth rth left a comment

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.

Thanks!

@rth
rth merged commit ac6dea5 into scikit-learn:main Mar 15, 2021
marrodion pushed a commit to marrodion/scikit-learn that referenced this pull request Mar 17, 2021
)

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@glemaitre glemaitre mentioned this pull request Apr 22, 2021
12 tasks
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