Skip to content

[MRG] TST arch-dependent divide-by-zero warning#10480

Merged
jnothman merged 3 commits intoscikit-learn:masterfrom
jnothman:armel
Feb 12, 2018
Merged

[MRG] TST arch-dependent divide-by-zero warning#10480
jnothman merged 3 commits intoscikit-learn:masterfrom
jnothman:armel

Conversation

@jnothman
Copy link
Copy Markdown
Member

Fixes #7572. @yarikoptic, I hope this solves the ARMEL issue.

Copy link
Copy Markdown
Member

@rth rth 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 @jnothman !

clean_warning_registry()
with warnings.catch_warnings(record=True) as w:
1. / np.array([0.])
numpy_provides_div0_warning = len(w) == 1
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.

Side comment: I imagine it could have been worth exposing the numpy_provides_div0_warning under some module in sklearn.utils if it was re-used elsewhere, but that doesn't seem to be the case now. Essentially it's a platform compatibility flag in line with other fixes in sklearn/utils/fixes.py.

@jnothman
Copy link
Copy Markdown
Member Author

Failure here is codecov. Can't be avoided, given that we do not run CI on armel. I assume this version makes you happier, @rth and @TomDLT?

Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Well, I was just thinking to move the corresponding binary flag to utils, but you are right this is probably cleaner and addresses the intended use case. LGTM

Minor doc comments below. Thanks @jnothman !

def assert_warns_div0(func, *args, **kw):
"""Assume that numpy's warning for bad divide is raised

Handles the case that platforms do not support warning on divide by zero
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.

the case of platforms that



def assert_warns_div0(func, *args, **kw):
"""Assume that numpy's warning for bad divide is raised
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.

"bad divide" -> "divide by zero"

@jnothman jnothman merged commit 8499a13 into scikit-learn:master Feb 12, 2018
@jnothman
Copy link
Copy Markdown
Member Author

thanks for the reviews

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