Skip to content
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

[MRG] Make cross_val_predict support method="predict_proba" and y=None #15918

Merged
merged 7 commits into from
Dec 23, 2019

Conversation

lkubin
Copy link
Contributor

@lkubin lkubin commented Dec 18, 2019

Reference Issues/PRs

Fixes #15855

Description

cross_val_predict raises exception when passing method="predict_proba" and y=None

What does this implement/fix? Explain your changes.

The problem is due to the fact that when method in ['decision_function', 'predict_proba', 'predict_log_proba'] y is cast to numpy array y = np.asarray(y) without checking if y is not None (when casting None to numpy array the singleton array array(None, dtype=object) is returned).

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

Please add a test

@lkubin lkubin changed the title [WIP] Fix #15855 [MRG] Fix #15855 Dec 19, 2019
@lkubin
Copy link
Contributor Author

lkubin commented Dec 19, 2019

@jnothman I think I'm done. Let me know if I missed something or the test I added doesn't look good

@jnothman
Copy link
Member

Please edit the pull request title to be more descriptive. It will become the commit message once merged.

Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

Please add an entry to the change log at doc/whats_new/v0.23.rst. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:

@lkubin
Copy link
Contributor Author

lkubin commented Dec 20, 2019

@jnothman I added the entry to the change log. Thanks for your support.

@lkubin lkubin changed the title [MRG] Fix #15855 [MRG] Make cross_val_predict support method="predict_proba" and y=None Dec 20, 2019
Copy link
Member

@thomasjpfan thomasjpfan left a comment

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 @lkubin !

sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
lkubin and others added 2 commits December 20, 2019 18:45
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
@lkubin
Copy link
Contributor Author

lkubin commented Dec 23, 2019

@thomasjpfan Thank you for your review. I think that I applied all the changes you were suggesting. Please let me know if I still miss something

@jnothman
Copy link
Member

Thanks!

@jnothman jnothman merged commit 88963f8 into scikit-learn:master Dec 23, 2019
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cross_val_predict raises error when method="predict_proba" and y=None
3 participants