Conversation
psomhorst
left a comment
There was a problem hiding this comment.
@JulietteFrancovich Sorry, I was confused about who wrote these tests, and should have assigned the issue this to myself. Thanks for the prompt response, though.
I don't see how mocking adds anything to already existing tests with real data. Let's make sure you get that data ASAP?
If you prefer, I can finish this PR.
| with pytest.raises(ValueError) as excinfo: | ||
| collection.combine(method="invalid") | ||
| assert "Unsupported method" in str(excinfo.value) |
There was a problem hiding this comment.
| with pytest.raises(ValueError) as excinfo: | |
| collection.combine(method="invalid") | |
| assert "Unsupported method" in str(excinfo.value) | |
| with pytest.raises(ValueError, match="Unsupported method): | |
| collection.combine(method="invalid") |
| # Test sum method | ||
| summed = collection.combine(method="sum") | ||
| assert isinstance(summed, PixelMask) | ||
|
|
||
| # Test product method | ||
| product = collection.combine(method="product") | ||
| assert isinstance(product, PixelMask) |
There was a problem hiding this comment.
Shouldn't these be done in another combine test? This function now tests three things that are not really related, while other tests already test the functioning of combine.
| assert "Unsupported data type" in str(excinfo.value) | ||
|
|
||
|
|
||
| def test_apply_to_eitdata_branch(): |
There was a problem hiding this comment.
What does this test add over test_apply_to_eitdata_labelled?
There was a problem hiding this comment.
For some reason test_apply_to_eitdata_labelled does not cover lines 308 and 309 in PixelMaskCollection.py:
308 case EITData(): 308 ↛ 309
309 return self._apply_mask_data(data, label_format,
This new test does but perhaps a better solution would be to understand why test_apply_to_eitdata_labelled doesn't....
There was a problem hiding this comment.
It does for me... Doesn't it skip/fail some tests because of missing data on your side?
There was a problem hiding this comment.
Oh check, it does. That explains (blonde moment)...
| with pytest.raises(TypeError) as excinfo: | ||
| collection.apply("not a valid type") | ||
| assert "Unsupported data type" in str(excinfo.value) |
There was a problem hiding this comment.
| with pytest.raises(TypeError) as excinfo: | |
| collection.apply("not a valid type") | |
| assert "Unsupported data type" in str(excinfo.value) | |
| with pytest.raises(TypeError, match="Unsupported data type"): | |
| collection.apply("not a valid type") |
I was confused too ;) but made a start. I think probably best if you finish the PR with tests with the real data (as the mocking was indeed a (sloppy) workaround). |
You can just unassigned yourself and assign it to me instead ;) |
No description provided.