Undeprecate ToGrayScale transforms and functionals #7122
Conversation
| ) | ||
| return Image.wrap_like(self, output) | ||
|
|
||
| def to_grayscale(self, num_output_channels: Literal[1, 3] = 1) -> Image: |
There was a problem hiding this comment.
Should the method be called rgb_to_grayscale() instead? Or just grayscale()?
Right now the class is called Grayscale(), the functionals are called rgb_to_grayscale...(). Keeping to_grayscale() for the methods seem like adding even more inconsistency, but IDK which one to go for.
We also have the to_grayscale() functional outlier in the v1 area which only works on PIL images (that we probably want to deprecate as discussed before).
There was a problem hiding this comment.
shoot, I merged before we got a chance to address that. Any thought @pmeier?
There was a problem hiding this comment.
I would keep consistency with the dispatchers, i.e. naming this rgb_to_grayscale.
|
Should be ready for a review now @pmeier |
|
|
||
| from torchvision.prototype import datapoints | ||
| from torchvision.prototype.transforms import functional as F, Transform | ||
| from typing_extensions import Literal |
There was a problem hiding this comment.
We should be consistent here. Sometimes you are using : int over Literal[1, 3]. I'm not going to oppose the removal the removal, but I personally would keep it given that it is going to become a language feature really soon: #7110 (comment)
There was a problem hiding this comment.
Literal isn't supported by torchscript. I had it everywhere, but had to remove it later on. LMK if you prefer int vs Literal for the non-JIT cases
There was a problem hiding this comment.
No, let's strife for consistency.
|
Hey @NicolasHug! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
Reviewed By: vmoens Differential Revision: D43116107 fbshipit-source-id: 0479e507930136a3eb0dd2ca761c795f07e1e004
3 participants
cc @bjuncek @pmeier