8000 [MRG+1] Return list instead of 3d array for MultiOutputClassifier.predict_proba by pjbull · Pull Request #8095 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG+1] Return list instead of 3d array for MultiOutputClassifier.predict_proba #8095

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Dec 22, 2016
Merged

[MRG+1] Return list instead of 3d array for MultiOutputClassifier.predict_proba #8095

merged 7 commits into from
Dec 22, 2016

Conversation

pjbull
Copy link
Contributor
@pjbull pjbull commented Dec 21, 2016

Reference Issue

Fixes #8093

What does this implement/fix? Explain your changes.

Returns a list of the predicted probabilities for each estimator in theMultiOutputClassifier rather than trying to stack them into a 3d matrix.

Any other comments?

This is the same behavior now as the RandomForestClassifier, which handles data of this shape without being wrapped in the MultiOutputClassifier.

jnothman
jnothman requested changes Dec 21, 2016
View reviewed changes
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.

Otherwise LGTM, thanks!

sklearn/multioutput.py
@@ -214,16 +214,18 @@ def predict_proba(self, X):

Returns
-------
T : (sparse) array-like, shape = (n_samples, n_classes, n_outputs)
The class probabilities of the samples for each of the outputs
p : array of shape = [n_samples, n_classes], or a list of n_outputs
Copy link
Member

Choose a reason for hiding this comment

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

for this type spec to render correctly as one line, you need a \ at the end of the line.

sklearn/tests/test_multioutput.py
@@ -150,6 +153,39 @@ def test_multiclass_multioutput_estimator():
list(predictions[:, i]))


def test_multiclass_multioutput_estimator_predict_proba():
# make test deterministic
rs = np.random.RandomState(123456)
Copy link
Member

Choose a reason for hiding this comment

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

By convention we use rng, not rs

@pjbull
Update flake8, docstring, variable name
b962c4a 
 - Changed `rs` to `rng` to follow convention.
 - Made sure changes were flake8 approved
 - Add `\` to continue docstring for `predict_proba` return value.
@pjbull
Copy link
Contributor Author
pjbull commented Dec 21, 2016

New commit has the requested changes.

@pjbull
Sub random.choice for np.random.choice
ddcdec2 
`np.random.choice` isn’t available in Numpy 1.6, so opt for the Python
version instead.
@pjbull
Make test labels deterministic
918abab
@pjbull
Copy link
Contributor Author
pjbull commented Dec 21, 2016

CI passing, should be good to go 🏇

TomDLT
TomDLT requested changes Dec 21, 2016
View reviewed changes
Copy link
Member
@TomDLT TomDLT left a comment

Choose a reason for hiding this comment

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

Remove sneaky log file and it's good to me

test_output.log
@@ -0,0 +1,1535 @@
rm -f tags
Copy link
Member 8000

Choose a reason for hiding this comment

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

you probably need to remove this file

jnothman
jnothman approved these changes Dec 22, 2016
View reviewed changes
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.

LGTM

@jnothman jnothman changed the title Return list instead of 3d array for MultiOutputClassifier.predict_proba [MRG+1] Return list instead of 3d array for MultiOutputClassifier.predict_proba Dec 22, 2016
@jnothman
Copy link
Member

Of course, after you remove the .log.

Please add an entry to what's new. Or even one under API changes and one under bug fixes.

@raghavrv
Copy link
Member

Thanks @pjbull... Merging once appveyor becomes green...

@raghavrv raghavrv merged commit dd2e48c into scikit-learn:master Dec 22, 2016
@jnothman
Copy link
Member
jnothman commented Dec 23, 2016 •
edited
Loading
8000

Thanks for identifying and fixing this, @pjbull

@pjbull pjbull deleted the multiclass-predict-proba branch December 23, 2016 14:03
sergeyf pushed a commit to sergeyf/scikit-learn that referenced this pull request Feb 28, 2017
@pjbull @sergeyf
[MRG+1] Return list instead of 3d array for MultiOutputClassifier.pre…
ec91436 
…dict_proba (scikit-learn#8095)

* Return list instead of 3d array for MultiOutputClassifier.predict_proba

* Update flake8, docstring, variable name

 - Changed `rs` to `rng` to follow convention.
 - Made sure changes were flake8 approved
 - Add `\` to continue docstring for `predict_proba` return value.

* Sub random.choice for np.random.choice

`np.random.choice` isn’t available in Numpy 1.6, so opt for the Python
version instead.

* Make test labels deterministic

* Remove hanging chad...

* Add bug fix and API change to whats new
@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
Closed
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
@pjbull @Sundrique
[MRG+1] Return list instead of 3d array for MultiOutputClassifier.pre…
124a25e 
…dict_proba (scikit-learn#8095)

* Return list instead of 3d array for MultiOutputClassifier.predict_proba

* Update flake8, docstring, variable name

 - Changed `rs` to `rng` to follow convention.
 - Made sure changes were flake8 approved
 - Add `\` to continue docstring for `predict_proba` return value.

* Sub random.choice for np.random.choice

`np.random.choice` isn’t available in Numpy 1.6, so opt for the Python
version instead.

* Make test labels deterministic

* Remove hanging chad...

* Add bug fix and API change to whats new
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
@pjbull @NelleV
[MRG+1] Return list instead of 3d array for MultiOutputClassifier.pre…
8453492 
…dict_proba (scikit-learn#8095)

* Return list instead of 3d array for MultiOutputClassifier.predict_proba

* Update flake8, docstring, variable name

 - Changed `rs` to `rng` to follow convention.
 - Made sure changes were flake8 approved
 - Add `\` to continue docstring for `predict_proba` return value.

* Sub random.choice for np.random.choice

`np.random.choice` isn’t available in Numpy 1.6, so opt for the Python
version instead.

* Make test labels deterministic

* Remove hanging chad...

* Add bug fix and API change to whats new
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
@pjbull @paulha
[MRG+1] Return list instead of 3d array for MultiOutputClassifier.pre…
791b063 
…dict_proba (scikit-learn#8095)

* Return list instead of 3d array for MultiOutputClassifier.predict_proba

* Update flake8, docstring, variable name

 - Changed `rs` to `rng` to follow convention.
 - Made sure changes were flake8 approved
 - Add `\` to continue docstring for `predict_proba` return value.

* Sub random.choice for np.random.choice

`np.random.choice` isn’t available in Numpy 1.6, so opt for the Python
version instead.

* Make test labels deterministic

* Remove hanging chad...

* Add bug fix and API change to whats new
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
@pjbull @maskani-moh
[MRG+1] Return list instead of 3d array for MultiOutputClassifier.pre…
dbcef9d 
…dict_proba (scikit-learn#8095)

* Return list instead of 3d array for MultiOutputClassifier.predict_proba

* Update flake8, docstring, variable name

 - Changed `rs` to `rng` to follow convention.
 - Made sure changes were flake8 approved
 - Add `\` to continue docstring for `predict_proba` return value.

* Sub random.choice for np.random.choice

`np.random.choice` isn’t available in Numpy 1.6, so opt for the Python
version instead.

* Make test labels deterministic

* Remove hanging chad...

* Add bug fix and API change to whats new
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnothman jnothman jnothman approved these changes

@TomDLT TomDLT TomDLT approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MultiOutputClassifier.predict_proba fails if targets have different number of values
4 participants
@pjbull @jnothman @raghavrv @TomDLT
0