[FIX] Improve ada boost widget #1787
Merged
Conversation
This file contains 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
pavlin-policar
force-pushed
the
improve-ada-boost-widget
branch
from
d9ad597
to
b7ed9e8
Compare
Current coverage is 88.96% (diff: 100%)@@ master #1787 diff @@
==========================================
Files 82 82
Lines 8963 8963
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 7974 7974
Misses 989 989
Partials 0 0
|
pavlin-policar
changed the title
lanzagar
requested changes
Dec 1, 2016
| self.base_estimator = self.DEFAULT_BASE_ESTIMATOR | ||
| else: | ||
| self.base_estimator = learner or self.DEFAULT_BASE_ESTIMATOR | ||
| self.base_label.setText( |
There was a problem hiding this comment.
This should be outside of the else block (remove one indent).
It sets the base_estimator correctly now, but not the label.
There was a problem hiding this comment.
Thanks, I missed that.
| from Orange.classification import SklTreeLearner, KNNLearner | ||
| from Orange.classification import KNNLearner | ||
| from Orange.classification import RandomForestLearner | ||
| from Orange.classification import SklTreeLearner |
There was a problem hiding this comment.
Grouping is preferred for imports like these (same as 2 lines below).
| @@ -1,5 +1,6 @@ | |||
| # Test methods with long descriptive names can omit docstrings | |||
| # pylint: disable=missing-docstring | |||
| from Orange.regression import RandomForestRegressionLearner | |||
lanzagar
requested changes
Dec 1, 2016
| self.assertTrue(self.widget.Error.no_weight_support.is_shown()) | ||
|
|
||
| def test_error_message_cleared_when_valid_learner_on_input(self): | ||
| # Disconneting an invalid learner should use the default one and hide |
| self.assertTrue(self.widget.Error.no_weight_support.is_shown()) | ||
|
|
||
| def test_error_message_cleared_when_valid_learner_on_input(self): | ||
| # Disconneting an invalid learner should use the default one and hide |
lanzagar
reviewed
Dec 1, 2016
| @@ -28,6 +29,11 @@ class OWAdaBoostClassification(OWBaseLearner): | |||
|
|
|||
| DEFAULT_BASE_ESTIMATOR = SklTreeLearner() | |||
|
|
|||
| class Error(OWBaseLearner.Error): | |||
There was a problem hiding this comment.
We decided it is best to have an Error message and require the user removes the invalid input before continuing.
The widget simply crashed if an invalid learner was set as its base estimator. Now, we use the widget Error messages and fail gracefully.
pavlin-policar
force-pushed
the
improve-ada-boost-widget
branch
from
a28e3c3
to
fa6c972
Compare
lanzagar
approved these changes
Dec 2, 2016
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.
Issue
AdaBoost simply crashed when a learner without support for
sample_weightswas put onto the input.Description of changes
This does not happen any more, but it uses the Orange widget Error that tells us that the learner doesn't support sample weights. I've also changed the control widths to match most of the other learner widgets.
Includes