[MRG+1] fixed OOB_Score bug for bagging classifiers. #8936
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mlewis1729
changed the title
mlewis1729
changed the title
mlewis1729
changed the title
jnothman
reviewed
May 26, 2017
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| assert_equal(bagging._max_samples, max_samples) | ||
|
|
||
|
|
||
| def test_set_oob_score(): |
There was a problem hiding this comment.
perhaps test_set_oob_score_label_encoding
There was a problem hiding this comment.
I can change the test name to that if it's preferred
|
yes, testing all is fine, but no need for random data. make it easy to read
the test
…On 26 May 2017 2:00 pm, "mlewis1729" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In sklearn/ensemble/tests/test_bagging.py
<#8936 (comment)>
:
> @@ -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():
+ # Make sure the oob_score doesn't change when the labels change
+ # See: #8933
+ N = 50
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.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#8936 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz64Q-B1iH2bDVI_NrLCsIip_-4-wmks5r9k5CgaJpZM4NmxZg>
.
|
jnothman
approved these changes
May 27, 2017
| 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 |
There was a problem hiding this comment.
Should avoid camel case for such variables
jnothman
changed the title
|
I'll fix in master the CamelCase and add a what's new entry thx @mlewis1729 |
|
see 6d2e9f2 |
Sundrique
pushed a commit
to Sundrique/scikit-learn
that referenced
this pull request
Jun 14, 2017
* 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
pushed a commit
to dmohns/scikit-learn
that referenced
this pull request
Aug 7, 2017
* 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
pushed a commit
to dmohns/scikit-learn
that referenced
this pull request
Aug 7, 2017
* 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
pushed a commit
to NelleV/scikit-learn
that referenced
this pull request
Aug 11, 2017
* 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
pushed a commit
to paulha/scikit-learn
that referenced
this pull request
Aug 19, 2017
* 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
pushed a commit
to AishwaryaRK/scikit-learn
that referenced
this pull request
Aug 29, 2017
* 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
pushed a commit
to maskani-moh/scikit-learn
that referenced
this pull request
Nov 15, 2017
* 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
* 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8933
Reference Issue
What does this implement/fix? Explain your changes.
Any other comments?