Port test/test_functional_tensor.py to pytest

[NicolasHug]

Currently, most tests in test/test_functional_tensor.py rely on unittest.TestCase. Now that we support pytest, we want to remove the use of the unittest module.

Instructions

There are many tests in this file, so I bundled them in multiple related groups below. If you're interested in working on this issue, please comment below with "I'm working on <group X>, <group Y>, etc..." so that others don't pick the same tests as you do. Feel free to pick as many groups as you wish, but please don't submit more than 2 groups per PR in order to keep the reviews manageable. Before picking a group, make sure it wasn't picked by another contributor first. Thanks!!

How to port a test to pytest

Porting a test from unittest to pytest is usually fairly straightforward. For a typical example, see https://github.com/pytorch/vision/pull/3907/files:

• take the test method out of the Tester(unittest.TestCase) class and just declare it as a function
• Replace @unittest.skipIf with pytest.mark.skipif(cond, reason=...)
• remove any use of self.assertXYZ.
  • Typically assertEqual(a, b) can be replaced by assert a == b when a and b are pure python objects (scalars, tuples, lists), and otherwise we can rely on assert_equal which is already used in the file.
  • self.assertRaises should be replaced with the pytest.raises(Exp, match=...): context manager, as done in https://github.com/pytorch/vision/pull/3907/files. Same for warnings with pytest.warns
  • self.assertTrue should be replaced with a plain assert
• When a function uses for loops to tests multiple parameter values, one should usepytest.mark.parametrize instead, as done e.g. in https://github.com/pytorch/vision/pull/3907/files.
• It may make sense to keep related tests within a single class. For example here, the tests in group A could be grouped into a TestToPILImage class, the tests in group N could be in TestPad, etc. Not all groups need a dedicated class though, it's on a case-by-case basis.
• Important: a lot of these tests rely on self.device because they need to be run on both CPU and GPU. For these, use the cpu_and_gpu() from common_utils instead, e.g.:

vision/test/test_functional_tensor.py

Lines 845 to 846 in f7b4cb0

	@pytest.mark.parametrize('device', cpu_and_gpu())
	def test_resize_asserts(device):

and you can just replace self.device by device in the test

CC @saswatpp as promised!

---

• Group A port tests in test_functional_tensor to pytest #3977
  • test_assert_image_tensor - parametrize over (func, arg)
  • test_vflip
  • test_hflip
  • test_crop -- parametrize over test_configs

(EDIT: oops, I initially named both groups below Group B. Renamed into B1 and B2)

• Group B1

  • test_hsv2rgb
  • test_rgb2hsv
  • test_rgb_to_grayscale -- parametrize over num_output_channels
  • test_center_crop
  • test_five_crop
  • test_ten_crop

• Group B2 port pad, resized_crop and affine in test_functional_tensor to pytest #3974

  • test_pad -- parametrize over dt, pad configs
  • test_resized_crop

• Group C port pad, resized_crop and affine in test_functional_tensor to pytest #3974
  This one might be a bit tricky. It will require some parametrization, and we should probably split each sub-method like _test_affine_all_ops, _test_affine_rect_rotations etc. into a single test function (or method, if you decide to bundle them into a TestAffine class, which would probably make sense).

  • test_affine

• Group D Port test_rotate in test_functional_tensor.py to pytest #3983
  Parametrize over data, dt, a, e, f, (and maybe c if there's no issue)
  We should split this test into multiple one, for example the self.assertWarnsRegex should be in a separate test

  • test_rotate

• Group E port tests in test_functional_tensor to pytest #3977
  parametrize over tensor, dt, ksize, sigma

  • test_gaussian_blur

cc @pmeier @vfdev-5

[saswatpp]

Thank you @NicolasHug ! I will try to add a PR for Group B and C.

[sahilg06]

@NicolasHug, I will try working on group A and E

[zhiqwang]

@NicolasHug, I will try working on Group D.

[sahilg06]

Hi @NicolasHug, in the function test_gaussian_blur in group E, tensor depends upon small image tensor and large image tensor and they depend upon the device. So How to parameterize tensor because I have to parameterize device also? please guide!

and 1 more question
which is the better way to parameterize?

1. test_configs =
   [
   (1, 2, 4, 5), # crop inside top-left corner
   (2, 12, 3, 4), # crop inside top-right corner
   (8, 3, 5, 6), # crop inside bottom-left corner
   (8, 11, 4, 3), # crop inside bottom-right corner
   ]

@pytest.mark.parametrize('top,left,height,width', test_configs)

2. @pytest.mark.parametrize('top,left,height,width',[(1, 2, 4, 5), # crop inside top-left corner
   (2, 12, 3, 4), # crop inside top-right corner
   (8, 3, 5, 6), # crop inside bottom-left corner
   (8, 11, 4, 3), # crop inside bottom-right corner
   ])

[NicolasHug]

So How to parameterize tensor because I have to parameterize device also? please guide!

We can probably parametrize over a new image_size parameter, like so:

@parametrize('image_size', ('small', 'large'))
def test_gaussian_blur(image_size, ...):
    if image_size == 'small':
        tensor = torch.from_numpy(
            np.arange(3 * 10 * 12, dtype="uint8").reshape((10, 12, 3))
        ).permute(2, 0, 1).to(device)
    else:
        tensor = torch.from_numpy(
            np.arange(26 * 28, dtype="uint8").reshape((1, 26, 28))
        ).to(device)

which is the better way to parameterize

I prefer the second way, unless test_configs can/should be re-used somewhere else.

[saswatpp]

@NicolasHug in Group C you mentioned that we should 'split' the sub-method into a 'single' function. I couldn't understand what you meant by that. And Since you have Split B into B1 and B2, let me take only B2 and someone in the community can take over B1.

[NicolasHug]

Sorry if that's not clear, basically I mean that instead of having a single function test_affine that calls many sub-functions as is the case right now, we should have multiple individual test functions like test_affine_all_ops, test_affine_rect_rotations, etc.

[sahilg06]

@NicolasHug this code is giving me error

with pytest.raises(Exception) as context:
        func(tensor, *args)

    assert 'Tensor is not a torch image.' in str(context.exception)

also I tried to write str(context) instead of str(context.exception) it followed with other errors

Help!

[NicolasHug]

I think you can just convert this into

with pytest.raises(Exception, match='Tensor is not a torch image'):
        func(tensor, *args)

[sahilg06]

@NicolasHug it is still giving the last 2 error for function five_crop() and center_crop(). actually some exception of positional arguments is occurring in these two functions which is not matching to "Tensor is not a torch image"

Hence 2 tests are failing and If something like this is happening, it should also happen to ten_crop() but that isn't the case

[NicolasHug]

Could you please submit a PR? It will be easier to see what's going on

[harishsdev]

@NicolasHug I am working on B1

[NicolasHug]

Thanks a lot for your interest @harishsdev but before working on B1, I would kindly ask that we first finish your work in #3975 as you're already working on 4 different groups. The CI is still failing there as the tests seem to loop indefinitely. As I already asked, please a) split the PR into 2 PRs and b) run the tests locally to avoid such issues. Don't hesitate to ask for specific help. Thanks!

[vivekkumar7089]

@NicolasHug , I wanted to work on the port for group B1. @harishsdev , if it is fine with you :)

[harishsdev]

@vivekkumar7089 please continue with B1

[NicolasHug]

Looks like we're done with this file, thank you so much everyone for your help!!
I'll close this issue, for those who are interested I opened a similar issue in #3987