8000 MNT silence spurious FitFailedWarning in test_search_cv by ogrisel · Pull Request #18833 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MNT silence spurious FitFailedWarning in test_search_cv #18833

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 3 commits into from
Dec 21, 2020
Merged

MNT silence spurious FitFailedWarning in test_search_cv #18833

merged 3 commits into from
Dec 21, 2020

Conversation

ogrisel
Copy link
Member
@ogrisel ogrisel commented Nov 13, 2020 •
edited
Loading

Reading the CI logs, there are extremely verbose FitFailedWarning triggered by running test_search_cv.

I think we should silence them, although I am not 100% sure this could not hide real problems.

Note there are also many SkipTestWarnings raised by common tests because of ColumnTransformer that is not default constructible but I am not sure what to do about that one.

@ogrisel ogrisel requested review from glemaitre and thomasjpfan and removed request for glemaitre November 13, 2020 14:31
@glemaitre
Copy link
Member

I think we should silence them, although I am not 100% sure this could not hide real problems.

I think that we can because we would not detect a failure currently.

@glemaitre
Copy link
Member

Note there are also many SkipTestWarnings raised by common tests because of ColumnTransformer that is not default constructible but I am not sure what to do about that one.

I think that we are raising this ourselves in parametrize_with_checks. We could take the option to only skip without raising a warning

glemaitre
glemaitre reviewed Nov 13, 2020
View reviewed changes
ogrisel
ogrisel commented Nov 13, 2020
View reviewed changes
@ogrisel ogrisel mentioned this pull request Nov 13, 2020
Merged
glemaitre
glemaitre approved these changes Dec 19, 2020
View reviewed changes
Copy link
Member
@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

It would be fine with me to merge this one

thomasjpfan
thomasjpfan approved these changes Dec 21, 2020
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

I am okay with this as well.

@thomasjpfan thomasjpfan merged commit dc56167 into scikit-learn:master Dec 21, 2020
@ogrisel ogrisel deleted the silence-FitFailedWarning branch December 22, 2020 11:04
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Dec 22, 2020
@ogrisel @thomasjpfan @glemaitre
MNT silence spurious FitFailedWarning in test_search_cv (scikit-learn…
585a3f0 
…#18833)

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@glemaitre glemaitre mentioned this pull request Dec 22, 2020
Merged
14 tasks
@glemaitre glemaitre mentioned this pull request Apr 22, 2021
Merged
12 tasks
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Apr 22, 2021
@ogrisel @thomasjpfan @glemaitre
MNT silence spurious FitFailedWarning in test_search_cv (scikit-learn…
77a938b 
…#18833)

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan thomasjpfan approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
Build / CI Waiting for Reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ogrisel @glemaitre @thomasjpfan
0