Always pass transform and target_transform to abstract dataset

[pmeier]

Kicked of by @zhiqwang (comment). Now, transforms, transform, and target_transform are passed to VisionDataset.__init__() for all datasets as intended.

@fmassa Should the usage of separate transforms deprecated? I'm asking since SBDataset only has joint transform:

vision/torchvision/datasets/sbd.py

Lines 52 to 57 in 8837e0e

	def __init__(self,
	root,
	image_set='train',
	mode='boundaries',
	download=False,
	transforms=None):

I'll fix the docstrings afterwards.

[fmassa]

Thanks for the PR! I'll have it reviewed tomorrow.

Just to answer your question:

@fmassa Should the usage of separate transforms deprecated? I'm asking since SBDataset only has joint transform:

I think we should still keep the separate transforms for now, as in most cases it's simpler for the user.

[pmeier]

I think we should still keep the separate transforms for now, as in most cases it's simpler for the user.

Should I add it to SBDataset?

[codecov-io]

Codecov Report

Merging #1126 into master will increase coverage by 0.34%.
The diff coverage is 56.66%.

@@            Coverage Diff             @@
##           master    #1126      +/-   ##
==========================================
+ Coverage   64.89%   65.24%   +0.34%     
==========================================
  Files          68       68              
  Lines        5413     5384      -29     
  Branches      835      835              
==========================================
  Hits         3513     3513              
+ Misses       1643     1615      -28     
+ Partials      257      256       -1

Impacted Files	Coverage Δ
torchvision/datasets/flickr.py	24.67% <0%> (+1.21%)	⬆️
torchvision/datasets/phototour.py	24.24% <0%> (+0.24%)	⬆️
torchvision/datasets/cifar.py	78.16% <100%> (-0.5%)	⬇️
torchvision/datasets/folder.py	82.05% <100%> (-0.45%)	⬇️
torchvision/datasets/mnist.py	50.71% <100%> (-0.47%)	⬇️
torchvision/datasets/svhn.py	66% <100%> (-1.31%)	⬇️
torchvision/datasets/lsun.py	21.59% <33.33%> (+0.93%)	⬆️
torchvision/datasets/caltech.py	21.59% <50%> (+0.93%)	⬆️
torchvision/datasets/sbu.py	23.72% <50%> (+0.77%)	⬆️
torchvision/datasets/omniglot.py	33.33% <50%> (+1.33%)	⬆️
... and 9 more

---

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 8837e0e...e1d776c. Read the comment docs.

[zhiqwang]

In the references directory, the dataset of CocoDetection also have this problem:

vision/references/detection/coco_utils.py

Line 209 in 8837e0e

def __init__(self, img_folder, ann_file, transforms):

Is it necessary to pass transforms (here is _transforms) to the constructor?

[fmassa]

@zhiqwang that is a custom dataset wrapper, and I on purpose not passed it to the constructor because I want the transforms to be applied after modifying the annotations to include the img_id, see

vision/references/detection/coco_utils.py

Line 216 in 8837e0e

target = dict(image_id=image_id, annotations=target)

Ideally I'd remove that and let the image id be returned by the dataloader, but that is currently not possible.

@pmeier

Should I add it to SBDataset?

It generally makes more sense to have joint transforms only because it are a segmentation dataset, and if you perform a transformation on the image you generally need to apply the same one to the target.
I would not be against adding it (for consistency), but I think it won't generally be that useful for this dataset.

[zhiqwang]

@fmassa Thanks for your explanation, and it help me understand the detection datasets a lot.

[fmassa]

Thanks for the PR @pmeier !

I think that we should keep the changes that passes the transform and target_transform to the constructor of VisionDataset, but I'm less sure of the benefits of adding transforms to all datasets, because that is generally not needed, and I'm not clear on which use-case this would come in handy.

Could you remove the added transforms in the constructor, and just pass the transform and target_transform?

[fmassa]

One more comment and then this is good to go, thanks a lot!

[fmassa]

LGTM, thanks a lot for you contribution!

Just waiting for CI to finish so that I can merge this