[MRG+1] fixed OOB_Score bug for bagging classifiers. #8936

Merged
merged 16 commits into from Jun 8, 2017

Conversation

Projects
None yet
3 participants
@mlewis1729
Contributor

mlewis1729 commented May 25, 2017

Fixes #8933

Reference Issue

What does this implement/fix? Explain your changes.

Any other comments?

Michael Lewis

@mlewis1729 mlewis1729 changed the title from fixed OOB_Score bug for bagging classifiers. to [MRG] fixed OOB_Score bug for bagging classifiers. May 25, 2017

Michael Lewis added some commits May 25, 2017

Michael Lewis
Michael Lewis
Michael Lewis

@mlewis1729 mlewis1729 changed the title from [MRG] fixed OOB_Score bug for bagging classifiers. to [WIP] fixed OOB_Score bug for bagging classifiers. May 25, 2017

Michael Lewis added some commits May 25, 2017

Michael Lewis
Michael Lewis
Michael Lewis
Michael Lewis
Michael Lewis

@mlewis1729 mlewis1729 changed the title from [WIP] fixed OOB_Score bug for bagging classifiers. to [MRG] fixed OOB_Score bug for bagging classifiers. May 26, 2017

sklearn/ensemble/tests/test_bagging.py
+def test_set_oob_score():
+ # Make sure the oob_score doesn't change when the labels change
+ # See: https://github.com/scikit-learn/scikit-learn/issues/8933
+ N = 50

This comment has been minimized.

@jnothman

jnothman May 26, 2017

Member

As a non-regression test, I don't see why this is any better than asserting that

BaggingClassifier(oob_score=True).fit([[0], [1]] * 5, ['A', 'B'] * 5).oob_score_

is approximately 1... or, at a minimum, not 0, which the current bug gives.

@jnothman

jnothman May 26, 2017

Member

As a non-regression test, I don't see why this is any better than asserting that

BaggingClassifier(oob_score=True).fit([[0], [1]] * 5, ['A', 'B'] * 5).oob_score_

is approximately 1... or, at a minimum, not 0, which the current bug gives.

This comment has been minimized.

@mlewis1729

mlewis1729 May 26, 2017

Contributor

I figured testing a "failing" label integer set ([-1,0,1]), a "standard" label set ([0,1,2]), and an alphanumeric label set (['a','b,'c']) would sufficiently exhaustive. Whatever your preference.

@mlewis1729

mlewis1729 May 26, 2017

Contributor

I figured testing a "failing" label integer set ([-1,0,1]), a "standard" label set ([0,1,2]), and an alphanumeric label set (['a','b,'c']) would sufficiently exhaustive. Whatever your preference.

sklearn/ensemble/tests/test_bagging.py
@@ -723,3 +723,37 @@ def test_max_samples_consistency():
max_features=0.5, random_state=1)
bagging.fit(X, y)
assert_equal(bagging._max_samples, max_samples)
+
+
+def test_set_oob_score():

This comment has been minimized.

@jnothman

jnothman May 26, 2017

Member

perhaps test_set_oob_score_label_encoding

@jnothman

jnothman May 26, 2017

Member

perhaps test_set_oob_score_label_encoding

This comment has been minimized.

@mlewis1729

mlewis1729 May 26, 2017

Contributor

I can change the test name to that if it's preferred

@mlewis1729

mlewis1729 May 26, 2017

Contributor

I can change the test name to that if it's preferred

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman May 26, 2017

Member
Member

jnothman commented May 26, 2017

Michael Lewis added some commits May 26, 2017

Michael Lewis
Michael Lewis
Michael Lewis
Michael Lewis
Michael Lewis
Michael Lewis
@jnothman

Otherwise lgtm

+def test_set_oob_score_label_encoding():
+ # Make sure the oob_score doesn't change when the labels change
+ # See: https://github.com/scikit-learn/scikit-learn/issues/8933
+ randState = 5

This comment has been minimized.

@jnothman

jnothman May 27, 2017

Member

Should avoid camel case for such variables

@jnothman

jnothman May 27, 2017

Member

Should avoid camel case for such variables

@jnothman jnothman changed the title from [MRG] fixed OOB_Score bug for bagging classifiers. to [MRG+1] fixed OOB_Score bug for bagging classifiers. May 27, 2017

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort Jun 8, 2017

Member

I'll fix in master the CamelCase and add a what's new entry

thx @mlewis1729

Member

agramfort commented Jun 8, 2017

I'll fix in master the CamelCase and add a what's new entry

thx @mlewis1729

@agramfort agramfort merged commit af41282 into scikit-learn:master Jun 8, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 94.88%)
Details
codecov/project 94.88% (+<.01%) compared to 9131f89
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@agramfort

This comment has been minimized.

Show comment
Hide comment
Member

agramfort commented Jun 8, 2017

see 6d2e9f2

Sundrique added a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

dmohns added a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

dmohns added a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

NelleV added a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

paulha added a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

AishwaryaRK added a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

[MRG+1] fixed OOB_Score bug for bagging classifiers. (#8936)
* fixed OOB_Score bug for bagging slassifiers.
See: scikit-learn#8933

* Added white space

* more white space fixing

* Adding test for oob_score validity

* removing pandas, replacing with numpy matrices

* fixing white space

* more white space fixing

* white space ...

* fixed labels to allow for strings

* white space

* simplifying test

* white space

* reformatting test

* white space

* pressed enter at end of file

* removing line at end of file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment