Skip to content

FIX update deprecated param for example using class_likelihood_ratios #30668

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 1 commit into from
Jan 20, 2025

Conversation

StefanieSenger
Copy link
Contributor

@StefanieSenger StefanieSenger commented Jan 18, 2025

This fixes a failure with the doc build on main that was accidentally introduced with #29288 (see comment #29288 (comment)).

Also, the PR makes class_likelihood_ratios use the new param when it's indirectly used in the scoring, so that users don't get the FutureWarning, when they have no control about it.

@lesteve @adrinjalali

Copy link

✔️ Linting Passed

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

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

@StefanieSenger StefanieSenger changed the title FIX update depracted param for example using class_likelihood_ratios FIX update deprecated param for example using class_likelihood_ratios Jan 18, 2025
@StefanieSenger
Copy link
Contributor Author

StefanieSenger commented Jan 18, 2025

The CI failures are related to OpenML being down. I believe the way we deal with it these weeks is to ignore these failures?

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

LGTM. But since the doc builds fail because of openml and this PR fixes doc stuff, I think it's safer to wait until we can have a completed doc build before merging to be sure we're not missing anything else.

@lesteve
Copy link
Member

lesteve commented Jan 20, 2025

I confirm running the examples locally that it fixes the warning from this particular example, so let's merge this and see whether that makes main happy.

About the change in sklearn/metrics/_scorer.py this kind of hints a lack of coverage in our tests (or that this is never used not sure).

@lesteve lesteve merged commit 119ade2 into scikit-learn:main Jan 20, 2025
32 of 35 checks passed
@jeremiedbb
Copy link
Member

About the change in sklearn/metrics/_scorer.py this kind of hints a lack of coverage in our tests (or that this is never used not sure).

never used indeed

@StefanieSenger StefanieSenger deleted the fix_examples branch January 20, 2025 10:46
@lesteve
Copy link
Member

lesteve commented Jan 20, 2025

Oh well it looks like main is even more red than before now (not related to this PR at all) ... because for some reason it can not find the datasets cache anymore ...

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