8000 MNT Catches scipy 1.3.0 for using deprecated tostring method by thomasjpfan · Pull Request #18755 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MNT Catches scipy 1.3.0 for using deprecated tostring method #18755

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 4 commits into from
Nov 6, 2020
Merged

MNT Catches scipy 1.3.0 for using deprecated tostring method #18755

merged 4 commits into from
Nov 6, 2020

Conversation

thomasjpfan
Copy link
Member

Reference Issues/PRs

Resolves #18752

What does this implement/fix? Explain your changes.

  1. Catches warnings before the pytest.warns can catch them for Voting*
  2. Filter out non convergence warnings for the rest.

@glemaitre glemaitre self-requested a review November 4, 2020 10:29
glemaitre
glemaitre approved these changes Nov 4, 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.

LGTM. I tested locally and the tests are not failing.

cmarmo
cmarmo approved these changes Nov 4, 2020
View reviewed changes
Copy link
Contributor
@cmarmo cmarmo left a comment

Choose a reason for hiding this comment

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

Thanks @thomasjpfan . Also DeprecationWarning are still output as error when the option -Werror::DeprecationWarning is activated.... I can't really understand why... but that's my problem... :)

@glemaitre
Copy link
Member

Also DeprecationWarning are still output as error when the option -Werror::DeprecationWarning is activated.... I can't really understand why... but that's my problem... :)

Uhm that's right. I did not add the flags when running the Python command. The thing is that @thomasjpfan manually filtered the ConvergenceWarning to make the count. With the flag, we would need to catch the DeprecationWarning.

@cmarmo
Copy link
Contributor
cmarmo commented Nov 4, 2020

Uhm that's right.

This is the expected behavior in the test suite so that's good...

ogrisel
ogrisel approved these changes Nov 6, 2020
View reviewed changes
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.

Looks good to me. The filtering seems specific enough and this is not a DeprecationWarning caused by scikit-learn itself.

@ogrisel ogrisel merged commit 8aa6f07 into scikit-learn:master Nov 6, 2020
@ogrisel
Copy link
Member
ogrisel commented Nov 6, 2020

Thanks @thomasjpfan and @cmarmo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@cmarmo cmarmo cmarmo approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
module:ensemble module:gaussian_process module:linear_model
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Failing tests due to scipy.optimize DeprecationWarnings
4 participants
@thomasjpfan @glemaitre @cmarmo @ogrisel
0