[POC] Base class for dataset tests

[pmeier]

Proof-of-concept for #3375.

We provide the fully-documented DatasetTestCase class that needs to be subclassed for each individual dataset. In most cases the user only has to configure the DATASET_CLASS to indicate which dataset should be tested and supply fake data within the inject_fake_data method. I've added tests for Caltech256, CIFAR10, and CIFAR100 to showcase this.

The user can add additional CONFIGS that are all tested instead of only the standard configuration. This can be used to test if the dataset works as intended for different splits or other parameter combinations.

---

Two things we need to discuss:

1. Some datasets require additional arguments without default values. For example

   vision/torchvision/datasets/ucf101.py

   Line 46 in 61e00d5

   def __init__(self, root, annotation_path, frames_per_clip, step_between_clips=1,

   vision/torchvision/datasets/coco.py

   Lines 49 to 52 in 61e00d5

   	def __init__(
   	self,
   	root: str,
   	annFile: str,

   We need an interface for the user to be able to provide them. My idea was to put this into inject_fake_data. In addition to the info dict, it could return a sequence of arguments passed to the constructor. If not returned, we could simply assume that only root should be passed, making this optional.

2. As of now, no tests for the (target_)transforms? is implemented. Writing the test is trivial, but we need a way to indicate which transform is applied to which output argument. Most of the time transform is applied to the first, target_transform to the second, and transforms to the first two. But there are exceptions to this:

   vision/torchvision/datasets/phototour.py

   Lines 106 to 109 in 61e00d5

   	if self.transform is not None:
   	data1 = self.transform(data1)
   	data2 = self.transform(data2)
   	return data1, data2, m[2]

   vision/torchvision/datasets/ucf101.py

   Lines 108 to 111 in 61e00d5

   	if self.transform is not None:
   	video = self.transform(video)
   	return video, audio, label

   My idea is to provide TRANSFORM_IDCS and TARGET_TRANSFORM_IDCS class attributes (TRANSFORMS_IDCS is not necessary as it can be combined from the other two). These would be mandatory, but could have good default values in (Image|Video)DatasetTestCase.

[pmeier]

I suspect the tests fail on Windows, because img = PIL.Image.open() only loads the image lazily. If I understand the documentation correctly, a user should call img.load() or any method that accesses the pixel data of the image after opening. After that the file that the data was loaded from can be deleted.

[codecov]

Codecov Report

Merging #3402 (3315ae5) into master (c1f85d3) will increase coverage by 0.30%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3402      +/-   ##
==========================================
+ Coverage   74.83%   75.13%   +0.30%     
==========================================
  Files         105      105              
  Lines        9722     9722              
  Branches     1563     1563              
==========================================
+ Hits         7275     7305      +30     
+ Misses       1960     1930      -30     
  Partials      487      487              

Impacted Files	Coverage Δ
torchvision/datasets/vision.py	65.51% <0.00%> (+1.72%)	⬆️
torchvision/datasets/cifar.py	90.00% <0.00%> (+13.75%)	⬆️
torchvision/datasets/caltech.py	42.52% <0.00%> (+20.68%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1f85d3...3315ae5. Read the comment docs.

[fmassa]

This looks great!

I have a few comments, specially about the handling of PIL opened images on Windows, let me know what you think.

About your questions:

1 - I might be missing something here, but why not leave those to be available in the config?

2 - Let's leave transform testing out for now. There might be a way to test that the transform is called without having to let the user specify it. Maybe by creating dummy transformer classes that return a specific dummy value, and checking the presence of this dummy value in the returned dataset.

[pmeier]

1 - I might be missing something here, but why not leave those to be available in the config?

I think that would be only reasonable if there is a good and independent default value:

vision/torchvision/datasets/mnist.py

Line 263 in c1f85d3

def __init__(self, root: str, split: str, **kwargs: Any) -> None:

The datasets I mentioned require an additional directory where the assumptions cannot be met. The annotation directory should be inside temporary directory which is only created shortly before inject_fake_data(). Furthermore, the additional directory should not be in root. For example, UCF101 collects the available classes by listing the contents of root:

vision/torchvision/datasets/ucf101.py

Line 58 in c1f85d3

classes = list(sorted(list_dir(root)))

Thus if the annotation directory is inside root, it will be recognized as class.

In most cases the temporary directory passed to inject_fake_data can be considered root, but as shown there are exceptions. For UCF101 the args should be something like root="/tmp/videos", annotation_root="/tmp/annotations".

---

2 - Let's leave transform testing out for now. There might be a way to test that the transform is called without having to let the user specify it. Maybe by creating dummy transformer classes that return a specific dummy value, and checking the presence of this dummy value in the returned dataset.

So instead of checking if the returned values are correct you only want to check if the (target_)?transforms? were called when accessing an example? That is not hard to implement. Note that this can only check if the (target_)?transforms? was called rather than what it was called with. For the latter we have the same issue as before, since without user interference we can't know which arguments should be passed to the transform.

[fmassa]

LGTM, thanks!