TST use global_dtype in sklearn/cluster/tests/test_affinity_propagation.py #22667
Conversation
jjerphan
changed the title
test_affinity_propagation.py to test implementations on 32bit datasets.
jeremiedbb
left a comment
jeremiedbb
left a comment
There was a problem hiding this comment.
I think _equal_similarities_and_preferences should be updated to use assert_allclose because it compares float arrays.
Co-authored-by: Jérémie du Boisberranger <[email protected]>
I don't think it should, because this is not a testing fixture but a utility function for affinity propagation that must not raise an |
Right. It should use np.allclose. But it would require to create a custom allclose with different rtols. Let's leave that for later |
|
|
||
|
|
||
| def test_equal_similarities_and_preferences(): | ||
| def test_equal_similarities_and_preferences(global_dtype): |
There was a problem hiding this comment.
The previous test (test_affinity_propagation_non_convergence_regressiontest) should raise the warning for the different dtypes I think?
There was a problem hiding this comment.
and test_affinity_propagation_poredict_non_convergence as well.
There was a problem hiding this comment.
The previous test (test_affinity_propagation_non_convergence_regressiontest) should raise the warning for the different dtypes I think?
Which difference of dtypes are you referring to?
I think this has been implemented in the meantime, hence I've merged |
cmarmo
added
the
Waiting for Second Reviewer
glemaitre
left a comment
glemaitre
left a comment
There was a problem hiding this comment.
LGTM. I just added the global_dtype to the 2 additional tests.
I am going to merge when it is green.
…on.py (scikit-learn#22667) Co-authored-by: Jérémie du Boisberranger <[email protected]> Co-authored-by: Guillaume Lemaitre <[email protected]>
5 participants
Reference Issues/PRs
Partially addresses #22881
Precedes #22590
What does this implement/fix? Explain your changes.
This parametrizes tests from
test_affinity_propagation.pyto run on 32bit datasets.Any other comments?
We could introduce a mechanism to be able to able to remove tests' execution on 32bit datasets if this takes too much time to complete.