Skip to content

MAINT use Voting and Stacking support missing values - #27710

Merged
thomasjpfan merged 6 commits into
scikit-learn:mainfrom
glemaitre:use_support_nan_voting_stacking
Dec 5, 2023
Merged

MAINT use Voting and Stacking support missing values#27710
thomasjpfan merged 6 commits into
scikit-learn:mainfrom
glemaitre:use_support_nan_voting_stacking

Conversation

@glemaitre

Copy link
Copy Markdown
Member

While working on #27709, it appears that using DecisionTree in common tests would be better because:

  • they support missing values
  • they handle multilabel classification
  • they handle multioutput regression

By changing the common tests, I came across an error regarding the allow_nan tag for both the Voting and Stacking estimators.

This PR makes 2 changes:

  • use decision tree in common tests to build the heterogeneous ensemble instance
  • fix allow_nan tags for Voting and Stacking estimators

@github-actions

github-actions Bot commented Nov 2, 2023

Copy link
Copy Markdown

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

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

@OmarManzoor OmarManzoor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @glemaitre

Comment thread sklearn/ensemble/_voting.py Outdated

@OmarManzoor OmarManzoor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @glemaitre

@thomasjpfan thomasjpfan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread sklearn/ensemble/_base.py
def _more_tags(self):
try:
allow_nan = all(
_safe_tags(est[1])["allow_nan"] if est[1] != "drop" else True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is better than main. Unfortunately, our tag system for allowing nans do not work with pipelines.

REF: #27903

@thomasjpfan
thomasjpfan merged commit fcdddd5 into scikit-learn:main Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants