MNT Use check_scalar in AdaBoostClassifier #21442
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
genvalen
changed the title
genvalen
changed the title
|
#DataUmbrella sprint |
…nto AdaBoostClassifier_add_check_scaler
genvalen
commented
Oct 26, 2021
Comment on lines
560
to
563
| def test_adaboost_classifier_params_validation(params, err_type, err_msg): | ||
| """Check the parameters validation in `AdaBoostClassifier`.""" | ||
| with pytest.raises(err_type, match=err_msg): | ||
| AdaBoostClassifier(**params).fit(X, y_class) # args are from toy sample |
There was a problem hiding this comment.
Hi @glemaitre, is it appropriate to test the estimator using the toy sample?
(lines 34-40)
# Toy sample
X = [[-2, -1], [-1, -1], [-1, -2], [1, 1], [1, 2], [2, 1]]
y_class = ["foo", "foo", "foo", 1, 1, 1] # test string class labels
y_regr = [-1, -1, -1, 1, 1, 1]
T = [[-1, -1], [2, 2], [3, 2]]
y_t_class = ["foo", 1, 1]
y_t_regr = [-1, 1, 1]Would you prefer if I generate different testing data?
There was a problem hiding this comment.
Not sure why this is marked as outdated, but I didn't make any changes to these lines yet
There was a problem hiding this comment.
Yes this is quite common. We should probably move those into a pytest.fixture for the full module to make it more explicit but I don't think you need to do that in this PR (it would be an effort for all test files).
There was a problem hiding this comment.
Great! That's good to know, thank you.
genvalen
changed the title
thomasjpfan
reviewed
Oct 29, 2021
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
glemaitre
changed the title
glemaitre
changed the title
glemaitre
reviewed
Oct 30, 2021
glemaitre
reviewed
Oct 30, 2021
glemaitre
reviewed
Oct 30, 2021
glemaitre
reviewed
Oct 30, 2021
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
…nto AdaBoostClassifier_add_check_scaler
|
Thanks @genvalen it looks good. |
|
Thank you for the feedback. :) |
samronsin
pushed a commit
to samronsin/scikit-learn
that referenced
this pull request
Nov 30, 2021
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
41 tasks
2 tasks
mathijs02
pushed a commit
to mathijs02/scikit-learn
that referenced
this pull request
Dec 27, 2022
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
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.
Reference Issues/PRs
Addresses #20724
#DataUmbrella
What does this implement/fix? Explain your changes.
Summary of changes to
AdaBoostClassifier:check_scalarfromsklearn.utilsto validate the scalar parameters.Progress:
References
1. docs
2. PR #20723
Any other comments?
I am new to these tests. Please make any suggestions. Thank you!