8000 SLEP006 FEA Add metadata routing support for `AdaBoostClassifier` and `AdaBoostRegressor` by adam2392 · Pull Request #29472 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

SLEP006 FEA Add metadata routing support for AdaBoostClassifier and AdaBoostRegressor #29472

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
Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

SLEP006 FEA Add metadata routing support for AdaBoostClassifier and AdaBoostRegressor #29472

wants to merge 3 commits into from

Conversation

adam2392
Copy link
Member
@adam2392 adam2392 commented Jul 11, 2024 •
edited
Loading

Reference Issues/PRs

Towards: #22893

What does this implement/fix? Explain your changes.

This is the final suite of estimators that needed metadata routing implemented.

AdaBoost* is a bit more complicated than other functions, or meta-estimators as they must pass sample_weight around internally. This means fit_params always contains sample_weight.

Any other comments?

I have a few questions:

  1. when should we deprecate, or remove sample_weight in function signatures?
  2. should all the methods including things like staged_predict have the **params signature get added?
  3. Inside get_metadata_routing, do I have to define when the callee is the self? E.g. in AdaBoostClassifier.predict_proba, it calls AdaBoostClassifier.decision_function.

@adam2392
WIP
52309bd 
Signed-off-by: Adam Li <adam2392@gmail.com>
@github-actions GitHub Actions
Copy link
github-actions bot commented Jul 11, 2024 •
edited
Loading

❌ Linting issues

This PR is introducing linting issues. Here's a summary of the issues. Note that you can avoid having linting issues by enabling pre-commit hooks. Instructions to enable them can be found here.

You can see the details of the linting issues under the lint job here

black

black detected issues. Please run black . locally and push the changes. Here you can see the detected issues. Note that running black might also fix some of the issues which might be detected by ruff. Note that the installed black version is black=24.3.0.


--- /home/runner/work/scikit-learn/scikit-learn/sklearn/tests/test_metaestimators_metadata_routing.py	2024-07-12 00:29:03.389250+00:00
+++ /home/runner/work/scikit-learn/scikit-learn/sklearn/tests/test_metaestimators_metadata_routing.py	2024-07-12 00:29:25.888177+00:00
@@ -422,11 +422,11 @@
             "decision_function",
             "score",
         ],
         "method_mapping": {"fit": ["fit", "score"]},
     },
-        {
+    {
         "metaestimator": AdaBoostRegressor,
         "estimator_name": "estimator",
         "estimator": "regressor",
         "X": X,
         "y": y,
would reformat /home/runner/work/scikit-learn/scikit-learn/sklearn/tests/test_metaestimators_metadata_routing.py

Oh no! 💥 💔 💥
1 file would be reformatted, 922 files would be left unchanged.

Generated for commit: b2b44b9. Link to the linter CI: here

adam2392 added 2 commits July 11, 2024 19:37
@adam2392
Almost there
911974c 
Signed-off-by: Adam Li <adam2392@gmail.com>
@adam2392
WIP
b2b44b9 
Signed-off-by: Adam Li <adam2392@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module:ensemble
Projects
Metadata routing
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@adam2392
0