Remove the use of assert_raises and assert_raises_regex from the tests #14649
Conversation
* from `sklearn/cluster/tests/test_affinity_propagation.py`
* from `sklearn/cluster/tests/test_bicluster.py`
* from `sklearn/cluster/tests/test_birch.py`
from `sklearn/cluster/tests/test_dbscan.py`
from `sklearn/cluster/tests/test_spectral.py`
* from `sklearn/cluster/tests/test_k_means.py`
glemaitre
left a comment
glemaitre
left a comment
There was a problem hiding this comment.
2 parametrizations can be done easily. Otherwise LGTM.
|
@glemaitre Could you take a look at this? Let me know if anything else needs to be done. |
NicolasHug
left a comment
NicolasHug
left a comment
There was a problem hiding this comment.
minimal comments but LGTM anyway.
Thanks for taking care of this @sameshl
|
|
||
| model = SpectralBiclustering(n_components=3, n_best=4) | ||
| assert_raises(ValueError, model.fit, data) | ||
| with pytest.raises(ValueError): |
There was a problem hiding this comment.
Add this one to the parametrization?
There was a problem hiding this comment.
@NicolasHug Can you explain how I can add this also in parametrization with already exsiting decorator?
There was a problem hiding this comment.
By adding {'n_components': 3, 'n_best': 4} to the list
There was a problem hiding this comment.
By adding
{'n_components': 3, 'n_best': 4}to the list
@NicolasHug I had some trouble making this parameterization. I already have
235 @pytest.mark.parametrize(
236 "args",
237 [{'n_clusters': (3, 3, 3)}, {'n_clusters': 'abc'},
238 {'n_clusters': (3, 'abc')}, {'method': 'unknown'},
239 {'n_components': 0}, {'n_best': 0},
240 {'svd_method': 'unknown'}]
241 )
242 def test_errors(args):
243 data = np.arange(25).reshape((5, 5))
244
245 model = SpectralBiclustering(**args)
246 with pytest.raises(ValueError):
247 model.fit(data)
248
249 model = SpectralBiclustering(n_components=3, n_best=4)
250 with pytest.raises(ValueError):
251 model.fit(data)
252
253 model = SpectralBiclustering()
254 data = np.arange(27).reshape((3, 3, 3))
255 with pytest.raises(ValueError):
256 model.fit(data)
Can you show me how the parameterization can be done?
There was a problem hiding this comment.
Here is how I would split this test:
@pytest.mark.parametrize(
"args",
[{'n_clusters': (3, 3, 3)},
{'n_clusters': 'abc'},
{'n_clusters': (3, 'abc')},
{'method': 'unknown'},
{'n_components': 0},
{'n_best': 0},
{'svd_method': 'unknown'},
{'n_components': 3, 'n_best': 4}]
)
def test_errors(args):
data = np.arange(25).reshape((5, 5))
model = SpectralBiclustering(**args)
with pytest.raises(ValueError):
model.fit(data)
def test_wrong_shape():
model = SpectralBiclustering()
data = np.arange(27).reshape((3, 3, 3))
with pytest.raises(ValueError):
model.fit(data)I added the {'n_components': 3, 'n_best': 4} case as a dict to the parametrization, and created a separate test function for the wrong shape test (it would be repeated for each case if we kept it in the original function)
There was a problem hiding this comment.
I understand now! Thanks for the nice explanation 😃
|
@NicolasHug I made the required changes. Let me know if anything else needs to be done |
|
Merging, thanks @sameshl ! |
|
Thanks for helping me out @NicolasHug! |
3 participants
This is a follow up on my PR #14645
This completes the replacement of
assert_raisesandassert_raises_regexwiththe
with pytest.raisescontext manager.related to #14216