F.pad is broken for PIL images and fill: str

[pmeier]

On the surface F.pad for PIL images seems to support str arguments for fill:

vision/torchvision/transforms/functional_pil.py

Lines 157 to 158 in 053e7eb

	if not isinstance(fill, (numbers.Number, str, tuple, list)):
	raise TypeError("Got inappropriate fill arg")

Internally though, it calls _parse_fill

vision/torchvision/transforms/functional_pil.py

Lines 304 to 308 in 053e7eb

	if img.mode != "F":
	if isinstance(fill, (list, tuple)):
	fill = tuple(int(x) for x in fill)
	else:
	fill = int(fill)

which in most cases relies on fill being numeric. Thus, actually passing a string doesn't work although PIL supports it.

>>> image = PIL.Image.fromarray(torch.randint(0, 256, (256, 256, 3), dtype=torch.uint8).numpy())
>>> F.pad(image, 128, fill="black")
ValueError: invalid literal for int() with base 10: 'black'
>>> PIL.ImageOps.expand(image, border=128, fill="black")
<PIL.Image.Image image mode=RGB size=512x512 at 0x7FA4DD37FBD0>

This traces back to #2284 and specifically to

vision/torchvision/transforms/functional.py

Lines 334 to 337 in 37a0d8d

	if len(fill) != len(img.getbands()):
	raise ValueError('fill should have the same number of elements '
	'as the number of channels in the image '
	'({}), got {} instead'.format(len(img.getbands()), len(fill)))

which will makes no sense for fill: str.

Given that #2284 was included from 0.7.0 upwards and we haven't gotten any report yet, my guess is that the functionality is not really used. Since we want to deprecate fill: str anyway (#5621 (comment)), I would opt to simply remove all mentions of it. Thoughts?

cc @vfdev-5 @datumbox

[datumbox]

That's indeed a very convenient bug. We should remove it rather than fix it, I agree. Nevertheless we should consider removing/deprecating string colours from the transforms not only for pad but also for all other transforms as these won't be compatible with the new API for non-PIL images.

[pmeier]

In total are two mentions of fill: str in transforms

vision/torchvision/transforms/transforms.py

Line 403 in 053e7eb

fill (number or str or tuple): Pixel fill value for constant fill. Default is 0. If a tuple of

vision/torchvision/transforms/transforms.py

Line 598 in 053e7eb

fill (number or str or tuple): Pixel fill value for constant fill. Default is 0. If a tuple of

vision/torchvision/transforms/functional.py

Line 451 in 9edd22c

fill (number or str or tuple): Pixel fill value for constant fill. Default is 0.

vision/torchvision/transforms/functional_pil.py

Lines 157 to 158 in 9edd22c

	if not isinstance(fill, (numbers.Number, str, tuple, list)):
	raise TypeError("Got inappropriate fill arg")

The only "non-pad" reference is on RandomCrop, which internally also use F.pad and thus is also broken.

[datumbox]

How about random erasing's value?

vision/torchvision/transforms/transforms.py

Lines 1653 to 1654 in 9edd22c

	if not isinstance(value, (numbers.Number, str, tuple, list)):
	raise TypeError("Argument value should be either a number or str or a sequence")

I suspect that string is not really supported there so worth checking.

There might be more like this. Worth having a deep check on the code to config these are the only references.

[pmeier]

Good catch. I only looked for fill. I'll have another look.

[pmeier]

Looking again for all str mentions, the only additional mention of a str representing a numeric value is RandomErasing as you found. Still, value: str is actually not correct here. It should be value: Literal["random"]:

vision/torchvision/transforms/transforms.py

Lines 1653 to 1656 in 9edd22c

	if not isinstance(value, (numbers.Number, str, tuple, list)):
	raise TypeError("Argument value should be either a number or str or a sequence")
	if isinstance(value, str) and value != "random":
	raise ValueError("If value is str, it should be 'random'")