Skip to content

MNT Use regression data for check_sample_weight_invariance test on multioutput regression metrics #30829

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.

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

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

lucyleeow
Copy link
Member

Reference Issues/PRs

Ref: #30787 (comment)

What does this implement/fix? Explain your changes.

test_multilabel_sample_weight_invariance used classification data for MULTIOUTPUT_METRICS.

Created a new test test_multioutput_sample_weight_invariance for MULTIOUTPUT_METRICS.

As "multilabel" does not accurately describe multioutput regression tasks, I didn't want to have this test under test_multilabel...

Any other comments?

I thought about changing the name to MULTIOUTPUT_REGRESSION_METRICS as we do use the term "Multiclass-multioutput" for classification (https://scikit-learn.org/dev/modules/multiclass.html#multiclass-and-multioutput-algorithms) BUT AFAICT we don't have any metrics that support "Multiclass-multioutput". WDYT @ogrisel ?

Copy link

✔️ Linting Passed

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

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

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

Lgtm, with either name.

Copy link
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

LGTM

@OmarManzoor OmarManzoor merged commit ebc1276 into scikit-learn:main Feb 14, 2025
39 of 40 checks passed
@lucyleeow lucyleeow deleted the fix_reg_test branch February 14, 2025 20:45
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