8000 [MRG] TST: fix scipy-dev-wheels build by lesteve · Pull Request #11359 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

[MRG] TST: fix scipy-dev-wheels build #11359

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
Jun 27, 2018

Conversation

lesteve
Copy link
Member
@lesteve lesteve commented Jun 26, 2018

Fix #11194 one more time ...

If this keeps happening I think we will need to have a better way of dealing with these.

I was expecting that pytest.warns would respect warnings.filters (the matrix subclass is not the recommended way is included in warnings.filters I double-checked) in contrary to sklearn.utils.testing.assert_no_warnings which internally does warnings.simplefilter('always') (+some custom ignoring of np.VisibleDeprecationWarning). Apparently my expectation about the interaction of pytest.warns and warnings.filters is not correct ...

8000

@lesteve lesteve changed the title TST: fix scipy-dev-wheels build. [MRG] TST: fix scipy-dev-wheels build Jun 26, 2018
Copy link
Member
@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

This looks good, but Travis is unhappy

Root cause is numpy.matrix PendingDeprecationWarning
@lesteve lesteve force-pushed the fix-scipy-dev-wheels branch from ca1ec4c to 6ec4866 Compare June 26, 2018 09:24
@lesteve
Copy link
Member Author
lesteve commented Jun 26, 2018

I fixed the flake8 error.

Copy link
Member
@TomDLT TomDLT 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 what about adding PendentDeprecationWarning in assert_no_warning, just like VisibleDeprecationWarning?

@lesteve
Copy link
Member Author
lesteve commented Jun 26, 2018

LGTM, but what about adding PendentDeprecationWarning in assert_no_warning, just like VisibleDeprecationWarning?

IIRC @jnothman mentioned somewhere (can't find the comment right now) that we should ignore warnings that comes from outside scikit-learn in our testing (or maybe only in assert_no_warnings not 100% sure). I had a quick look at using something like this in assert_no_warnings but I never got around to actually trying it out:

warnings.filterwarning('always', module='^sklearn')
warnings.simplefilter('ignore')

@jnothman
Copy link
Member
jnothman commented Jun 26, 2018 via email

@lesteve
Copy link
Member Author
lesteve commented Jun 27, 2018

I would be in favour of merging this PR as is, similarly to what was done for the previous one (#11251). Although not ideal, I think this does fix the scipy-dev-wheels build.

@jnothman jnothman merged commit b89c83f into scikit-learn:master Jun 27, 2018
@lesteve lesteve deleted the fix-scipy-dev-wheels branch June 28, 2018 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0