Skip to content

MAINT Fix assert raises in sklearn/semi_supervised/tests/#14841

Merged
thomasjpfan merged 2 commits intoscikit-learn:masterfrom
sameshl:tests_semi_supervised
Sep 8, 2019
Merged

MAINT Fix assert raises in sklearn/semi_supervised/tests/#14841
thomasjpfan merged 2 commits intoscikit-learn:masterfrom
sameshl:tests_semi_supervised

Conversation

@sameshl
Copy link
Copy Markdown
Contributor

@sameshl sameshl commented Aug 29, 2019

replaced assert_raises and assert_raises_regex with pytest.raises context manager.

related to #14216

@sameshl
Copy link
Copy Markdown
Contributor Author

sameshl commented Sep 6, 2019

@rth Could you have a look at this?
Thanks!

label_propagation.LabelSpreading(**kwargs).fit(X, y),
alpha=alpha)
with pytest.raises(ValueError):
(lambda **kwargs:
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.

That is a weird use case for lambda. This should also work:

label_propagation.LabelSpreading(alpha=alpha).fit(X, y)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@thomasjpfan I have corrected it. Let me know if anything else needs to be done.

Copy link
Copy Markdown
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.

LGTM

@thomasjpfan thomasjpfan changed the title MAINT:Fix assert raises in sklearn/semi_supervised/tests/ MAINT Fix assert raises in sklearn/semi_supervised/tests/ Sep 8, 2019
@thomasjpfan thomasjpfan merged commit 3aafaa9 into scikit-learn:master Sep 8, 2019
@thomasjpfan
Copy link
Copy Markdown
Member

Thank you @sameshl !

@sameshl sameshl deleted the tests_semi_supervised branch September 8, 2019 02:03
@sameshl
Copy link
Copy Markdown
Contributor Author

sameshl commented Sep 8, 2019

Thanks @thomasjpfan!

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.

2 participants