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+1] Resolve the problem with cross_val_predict(method=) when passing X or y as list #9600

Merged
merged 7 commits into from Aug 22, 2017

Conversation

@rrkarim
Copy link

@rrkarim rrkarim commented Aug 22, 2017

Reference Issue

Fixes: #9592

@jnothman
Copy link
Member

@jnothman jnothman commented Aug 22, 2017

Please make the title more descriptive, and the reference issue should be prefixed by "Fixes"

@rrkarim rrkarim changed the title issue 9592 Resolve the problem with cross_val_predict(method=) when passing X or y as list Aug 22, 2017
Rasul Kerimov added 2 commits Aug 22, 2017
Rasul Kerimov
Rasul Kerimov
@@ -732,7 +732,7 @@ def _fit_and_predict(estimator, X, y, train, test, verbose, fit_params,
predictions = func(X_test)
if method in ['decision_function', 'predict_proba', 'predict_log_proba']:
n_classes = len(set(y))
predictions_ = np.zeros((X_test.shape[0], n_classes))
predictions_ = np.zeros((((X_test.shape[0]) if type(X_test) is np.ndarray else (_num_samples(X_test))), n_classes))

This comment has been minimized.

@jnothman

jnothman Aug 22, 2017
Member

you should be able to use _num_samples in all cases.

This comment has been minimized.

@rrkarim

rrkarim Aug 22, 2017
Author

true...

@@ -808,6 +808,10 @@ def test_cross_val_predict_input_types():
clf = CheckingClassifier(check_y=list_check)
predictions = cross_val_predict(clf, X, y.tolist())

#test with X and y as list and non empty method
predictions = cross_val_predict(LogisticRegression(), X.tolist(), y.tolist(), method='decision_function')

This comment has been minimized.

@jnothman

jnothman Aug 22, 2017
Member

Please keep line length under 80 chars

rrkarim and others added 2 commits Aug 22, 2017
Rasul Kerimov
Copy link
Member

@jnothman jnothman left a comment

Thanks

@@ -808,6 +808,12 @@ def test_cross_val_predict_input_types():
clf = CheckingClassifier(check_y=list_check)
predictions = cross_val_predict(clf, X, y.tolist())

#test with X and y as list and non empty method

This comment has been minimized.

@jnothman

jnothman Aug 22, 2017
Member

PEP8: space after #

@@ -808,6 +808,12 @@ def test_cross_val_predict_input_types():
clf = CheckingClassifier(check_y=list_check)
predictions = cross_val_predict(clf, X, y.tolist())

#test with X and y as list and non empty method
predictions = cross_val_predict(LogisticRegression(), X.tolist(),

This comment has been minimized.

@jnothman

jnothman Aug 22, 2017
Member

PEP8 indentation, please; preferably:

    predictions = cross_val_predict(LogisticRegression(), X.tolist(),
                                    y.tolist(), method='decision_function')
@jnothman
Copy link
Member

@jnothman jnothman commented Aug 22, 2017

LGTM, thanks!

@jnothman jnothman changed the title Resolve the problem with cross_val_predict(method=) when passing X or y as list [MRG+1] Resolve the problem with cross_val_predict(method=) when passing X or y as list Aug 22, 2017
@jnothman jnothman added this to the 0.19.1 milestone Aug 22, 2017
@jnothman jnothman added the Bug label Aug 22, 2017
@NelleV
Copy link
Member

@NelleV NelleV commented Aug 22, 2017

Thanks @CoderINusE !

@NelleV NelleV merged commit 9e9b78d into scikit-learn:master Aug 22, 2017
6 checks passed
6 checks passed
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.16%)
Details
codecov/project 96.16% (+<.01%) compared to 6f42105
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python 2 new alerts
Details
jnothman added a commit to jnothman/scikit-learn that referenced this pull request Aug 23, 2017
…ing X or y as list (scikit-learn#9600)

* issue 9592

* issue resolve

* resolve issue

* review

* Delete sample.py

* review
AishwaryaRK added a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017
…ing X or y as list (scikit-learn#9600)

* issue 9592

* issue resolve

* resolve issue

* review

* Delete sample.py

* review
maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
…ing X or y as list (scikit-learn#9600)

* issue 9592

* issue resolve

* resolve issue

* review

* Delete sample.py

* review
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
…ing X or y as list (scikit-learn#9600)

* issue 9592

* issue resolve

* resolve issue

* review

* Delete sample.py

* review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants