Port affine transform tests in test_tranforms to pytest #3984
Conversation
There was a problem hiding this comment.
thanks @Ishan-Kumar2 , I made a few comments but this looks good!
test/test_transforms.py
Outdated
| result_matrix[2, 2] = 1 | ||
| return np.linalg.inv(result_matrix) | ||
|
|
||
| def _test_transformation(self, a, t, s, sh, PIL_Image): |
There was a problem hiding this comment.
here and in the rest of the code, let's take this opportunity to use better names: angle, translate, scale, shear, etc.
test/test_transforms.py
Outdated
| @pytest.mark.parametrize("pt", [(16, 16), (20, 16), (20, 20)]) | ||
| @pytest.mark.parametrize("i", range(-5, 5)) | ||
| @pytest.mark.parametrize("j", range(-5, 5)) | ||
| def test_make_img_affine(self, pt, i, j): | ||
| self.input_img[pt[0] + i, pt[1] + j, :] = [255, 155, 55] |
There was a problem hiding this comment.
Hm, this almost works 😄
The problem is that this assumes that test_make_img_affine is called before the rest of the tests, which isn't an assumption we should make. Also, filling the values of the input_img isn't a test in itself so we should use a different logic here.
This is a good case for using a fixture https://docs.pytest.org/en/6.2.x/fixture.html:
@pytest.fixture(scope='class')
def input_img(self):
out = np.zeros((40, 40, 3), dtype=np.uint8)
for pt in ...
for i in ...
for j in ...
...
return outand then we can use input_img in PIL_Image directly
There was a problem hiding this comment.
Haha yeah it makes sense. Guess I got carried away by pytest's parameterisation 😆
test/test_transforms.py
Outdated
| F.affine(self.input_img, 10, translate=0, scale=1, shear=1) | ||
|
|
||
| @pytest.fixture | ||
| def PIL_Image(self): |
There was a problem hiding this comment.
Here and everywhere else, let's rename this into pil_image as the current case suggests that this is a class instead of an instance
|
@NicolasHug thanks a lot for the feedback and detailed review. I have made the changes. Let me know if anything else needs to be updated :) |
Reviewed By: fmassa Differential Revision: D29097725 fbshipit-source-id: 8913fc8e9f8f11beb1de512c22807c8bd39690c0
Refactor Group C as mentioned in #3945
@NicolasHug I converted the test_affine function into a class so as to optimize the parameterisation. Let me know if the names of the functions need to be changed.