8000 MAINT Deprecate scoring='max_error' and replace it by scoring='neg_max_error' by artificialfintelligence · Pull Request #29462 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MAINT Deprecate scoring='max_error' and replace it by scoring='neg_max_error' #29462

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
Merged
merged 10 commits into from
Jul 19, 2024
Merged

MAINT Deprecate scoring='max_error' and replace it by scoring='neg_max_error' #29462

merged 10 commits into from
Jul 19, 2024

Conversation

artificialfintelligence
Copy link
Contributor
@artificialfintelligence artificialfintelligence commented Jul 11, 2024 •
edited by lesteve
Loading

Reference Issues/PRs

Fixes #29417 8000

This deprecates scoring='max_error and replaces it with scoring='neg_max_error'.

This is very similar to #14898 that did a similar thing for scoring='brier_score_loss' => scoring='neg_brier_score'

What does this implement/fix? Explain your changes.

Renames the max_error scorer to neg_max_error in order to make it consistent with other scorers that have greater_is_better = False (and consistent with the documentation as well). Deprecates max_error with a deprecation warning message stating that it will be removed in v1.8.

Any other comments?

Please remove the deprecation warning, deprecation test case and the two comment lines I added (which start with XXX) once the old scorer is removed.

@github-actions GitHub Actions
Copy link
github-actions bot commented Jul 11, 2024 •
edited
Loading

✔️ Linting Passed

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

Generated for commit: 0e53cce. Link to the linter CI: here

@lesteve lesteve changed the title Issue/13887 MAINT Deprecate scoring='max_error' and replace it by scoring='neg_max_error' Jul 11, 2024
@lesteve
Copy link
Member
lesteve commented Jul 11, 2024

Thanks for your PR, I edited your description and PR title to help potential reviewers!

lesteve
lesteve reviewed Jul 17, 2024
View reviewed changes
lesteve
lesteve reviewed Jul 17, 2024
View reviewed changes
lesteve
lesteve reviewed Jul 17, 2024
View reviewed changes
lesteve
lesteve reviewed Jul 18, 2024
View reviewed changes
lesteve
lesteve approved these changes Jul 18, 2024
View reviewed changes
Copy link
Member
@lesteve lesteve left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@artificialfintelligence @lesteve
Better comments
0e53cce 
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
adrinjalali
adrinjalali approved these changes Jul 19, 2024
View reviewed changes
Copy link
Member
@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

LGTM, and it seems consistent with the other work we've done.

@adrinjalali adrinjalali merged commit 1bcddcb into scikit-learn:main Jul 19, 2024
30 checks passed
This was referenced Jan 23, 2025
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali approved these changes

@lesteve lesteve lesteve approved these changes

Assignees
No one assigned
Labels
module:metrics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Documentation section 3.4.1.1 has incorrect description that would be correct if the max_loss metric were to be tweaked and renamed
3 participants
@artificialfintelligence @lesteve @adrinjalali
0