TST Extend tests for scipy.sparse.*array in sklearn/preprocessing/tests/test_label.py#27227
Conversation
OmarManzoor
left a comment
There was a problem hiding this comment.
Thanks for the PR @Tialo
| classes=[1, 2], | ||
| threshold=0, | ||
| ) | ||
| for csr_container in CSR_CONTAINERS: |
There was a problem hiding this comment.
How about splitting the two loops that are using spare containers into a separate test and then parametrizing that? I think these tests are not dependent on anything else within this test.
There was a problem hiding this comment.
Can't this loop also be moved into the added test test_label_binarizer_sparse_errors?
|
|
||
| with pytest.raises(ValueError): | ||
| mlb.inverse_transform(csr_matrix(np.array([[0, 1, 1], [2, 0, 0], [1, 1, 0]]))) | ||
| for csr_container in CSR_CONTAINERS: |
There was a problem hiding this comment.
Similarly you might also be able to move this into a separate test and parametrize that since this part is just checking for an error and is independent from the rest of the test.
|
@OmarManzoor Thanks for your comments. In second test I used MultiLabelBinarizer and input according to last values in for loop, because it was used in original test, if you have better solution, I will be glad to make a fix |
I think it looks fine considering it is replicating the original behavior. 👍 |
|
@OmarManzoor if you want to have a second look at it. |
…tests/test_label.py` (scikit-learn#27227)
Reference Issues/PRs
Towards #27090.
What does this implement/fix? Explain your changes.
Any other comments?
I used for loops instead of parameterisation in some tests to save time and do not repeat other checks in test that do not depend on sparse type.
Also i fixed test
test_label_binarize_multilabel. It used only lastyfrom for loop inpytest.raises(ValueError)