-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Updated the base.OutlierMixin.fit_predict method docstring #12954
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
Updated the base.OutlierMixin.fit_predict method docstring #12954
Conversation
…ze that it performs a fit on X before returning the predictions on X
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 clarification.
Note that you always have |
@finnoshea have you created an issue for this PR? It's always good idea to open an issue before submitting a PR. |
Ohh yeah, now that I've looked through the code I see that anything that inherits from OutlierMixin (at the very least) will have that functionality. But I spent way too long trying to squash a bug because of my misunderstanding of what fit_predict did. |
@nixphix I don't think there's anything wrong with just creating a pull request for this kind of change |
Thanks @finnoshea |
…string (scikit-learn#12954)" This reverts commit 27bac67.
…string (scikit-learn#12954)" This reverts commit 27bac67.
to emphasze that it performs a fit on X before returning the predictions on X
Reference Issues/PRs
What does this implement/fix? Explain your changes.
The docstring for OutlierMixin.fit_predict in sklearn/base.py only says that it makes predictions on X and not that it also performs a fit first. I have modified the docstring to show that it does perform the fit first. Somewhere I got the idea that fit_predict was for predictions on the data that was fit, but it is obviously not.
Any other comments?