-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
ENH Make KNeighborsClassifier.predict
handle X=None
#30047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
KNeighborsClassifier.predict
handle X=None
KNeighborsClassifier.predict
handle X=None
There was a problem hiding this 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 @dkobak
Co-authored-by: Omar Salman <omar.salman2007@gmail.com>
@OmarManzoor After I commited your one-line change to |
It seems like there are some conflicts with main. Can you merge main in your branch and resolve the conflicts? |
Oh! Turns out that upcoming changes should not go into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @dkobak
Reference Issues/PRs
Fixes #29722 and #27747.
This makes
predict()
,predict_proba()
, andscore()
ofKNeighborsClassifier
andRadiusNeighborsClassifier
takeX=None
. In this case predictions for all training set points are returned, and points are NOT included into their own neighbors. This makes it consistent with the current behaviour ofKNeighborsRegressor
andRadiusNeighborsRegressor
.New functionality is described in docstrings (and added to the docstrings of
KNeighborsRegressor
andRadiusNeighborsRegressor
).Tests are added confirming that
knn.fit(X, y).predict(None)
is equivalent tocross_val_predict(knn, X, y, cv=LeaveOneOut())
.Sanity checking
@ogrisel asked me to add a notebook showing that this is a statistically sane thing to do (#29722 (comment)):
I did this here: #29722 (comment). Is it sufficient to simply leave it there?