Skip to content

Deprecate cv_values_ in favour of cv_results_ in RidgeCV estimators#28915

Merged
jeremiedbb merged 6 commits into
scikit-learn:mainfrom
lucyleeow:cv_results
May 2, 2024
Merged

Deprecate cv_values_ in favour of cv_results_ in RidgeCV estimators#28915
jeremiedbb merged 6 commits into
scikit-learn:mainfrom
lucyleeow:cv_results

Conversation

@lucyleeow

@lucyleeow lucyleeow commented Apr 30, 2024

Copy link
Copy Markdown
Member

Reference Issues/PRs

closes #7206

What does this implement/fix? Explain your changes.

Deprecate cv_values_ and store_cv_values in RidgeCV and RidgeClassifierCV

Any other comments?

@lucyleeow lucyleeow changed the title Deprevate cv_values_ in favour of cv_results_ Deprecate cv_values_ in favour of cv_results_ Apr 30, 2024
@lucyleeow lucyleeow changed the title Deprecate cv_values_ in favour of cv_results_ Deprecate cv_values_ in favour of cv_results_ in RidgeCV estimators Apr 30, 2024
@github-actions

github-actions Bot commented Apr 30, 2024

Copy link
Copy Markdown

✔️ Linting Passed

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

Generated for commit: 3cdbcf7. Link to the linter CI: here

@adrinjalali adrinjalali 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.

Thanks for the PR @lucyleeow . Also need to trigger a doc build to make sure we don't have this in our docs.

Comment thread sklearn/linear_model/_ridge.py Outdated
Comment thread sklearn/linear_model/_ridge.py
Comment thread sklearn/linear_model/_ridge.py Outdated
Comment thread sklearn/linear_model/_ridge.py Outdated
Comment thread sklearn/linear_model/_ridge.py Outdated

@adrinjalali adrinjalali 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.

Otherwise LGTM.

cc @jeremiedbb maybe?

Comment thread sklearn/linear_model/_ridge.py Outdated
Comment thread sklearn/linear_model/_ridge.py Outdated

@jeremiedbb jeremiedbb 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. There's just a warning message mismatch. And +1 to mark None as Hidden (we won't start a deprecation cycle in 1.7 to remove None)

Comment thread sklearn/linear_model/tests/test_ridge.py Outdated
@jeremiedbb jeremiedbb enabled auto-merge (squash) May 2, 2024 14:04
@jeremiedbb jeremiedbb merged commit 4e20b01 into scikit-learn:main May 2, 2024
@lucyleeow lucyleeow deleted the cv_results branch May 2, 2024 23:25
@lucyleeow

Copy link
Copy Markdown
Member Author

Thanks for the review!

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.

results_ attribute for all EstimatorCV classes?

3 participants