Port test/test_transforms #3958
Conversation
|
PR for #3945 |
test/test_transforms.py
Outdated
| @@ -681,15 +681,224 @@ def test_convert_image_dtype_int_to_int_consistency(self): | |||
| self.assertEqual(actual_min, desired_min) | |||
| self.assertEqual(actual_max, desired_max) | |||
|
|
|||
| @unittest.skipIf(accimage is None, 'accimage not available') | |||
|
|
|||
| def test_affine(self): | |||
There was a problem hiding this comment.
We should try to move these methods out of the class and just have pure functions at the bottom of the file. In the code below there are a lot of references to self which we want to remove and replace as per the instrucitons in #3945 :)
test/test_transforms.py
Outdated
| for pt in [(16, 16), (20, 16), (20, 20)]: | ||
| for i in range(-5, 5): | ||
| for j in range(-5, 5): |
There was a problem hiding this comment.
these 3 for-loops is typically the kind of thing that we will want to parametrize, as done for example in https://github.com/pytorch/vision/pull/3907/files
|
I will open NEW PR as i am facing many issues for PEP8 |
|
@harishsdev thanks for your work so far. It's usually better to just stick to a single PR: opening many PRs makes it harder for us to keep track of the changes and of the reviews. It's totally fine to push many fix commit in a given PR: we'll squash them all when merging anyway so all commits will disappear. |
| with pytest.raises(ValueError): | ||
| transforms.RandomAffine(-0.7) |
There was a problem hiding this comment.
We'll need 1 raises block per line here, because we need to make sure that each individual line raises a ValueError. That's not your fault: the original code was wrong I think
|
|
||
| def test_random_affine(): |
There was a problem hiding this comment.
Please write these new tests at the end of the file instead of at the beginning, it will be easier to find the remaining tests that need to be ported :)
| @pytest.mark.parametrize(i,range(-5,5)) | ||
| @pytest.mark.parametrize(j,range(-5,5)) | ||
|
|
||
| def test_affine('a','t1','s','s',i,j): |
There was a problem hiding this comment.
this should be a, t1, s, etc., without the quotes as they're parameter names
| @pytest.mark.parametrize('t1',range(-10, 10, 5)) | ||
| @pytest.mark.parametrize('s',[0.77, 1.0, 1.27]) | ||
| @pytest.mark.parametrize('s1',range(-15, 15, 5)) | ||
| @pytest.mark.parametrize(pt,[(16, 16), (20, 16), (20, 20)]) |
There was a problem hiding this comment.
this should be 'pt' with quotes and you'll need a space after the comma ;)
(here and in other places)
|
okay,Thanks @NicolasHug ,please help to reopen this PR |
|
Unfortunately it seems that you deleted your fork (or your branch?) so I can't reopen. That's fine, we can open a new PR, but please make sure to take a look at my comments above :) |
PR for below porting pytest code
Group B
test_accimage_crop
test_accimage_pil_to_tensor
test_accimage_resize
test_accimage_to_tensor
Group C
test_affine
test_random_affine