Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a GrayscaleToRgb transform that can expand channels to 3 #8247

Merged
merged 17 commits into from
Mar 15, 2024

Conversation

ahmadsharif1
Copy link
Contributor

@ahmadsharif1 ahmadsharif1 commented Feb 1, 2024

This is for issue #8186.

GrayscaleToRgb transform can take a grayscale image and expand it to 3 channels, with the same values for R, G and B.

cc @vfdev-5

Copy link

pytorch-bot bot commented Feb 1, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8247

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit b2daebd with merge base fa82fd3 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @ahmadsharif1 ! I think there's a small issue where the images are in fact converted to grayscale instead of being preserved as RGB

torchvision/transforms/v2/functional/_color.py Outdated Show resolved Hide resolved
torchvision/transforms/transforms.py Outdated Show resolved Hide resolved
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @ahmadsharif1 , just one minor comment below but this looks good. Maybe you can undraft the PR?
Now it's time to bikeshed about the name. I'm tempted to just call that ToRGB() instead of GrayScaleToRGB() so that it can be called at the beginning of every pipeline without suggesting that the input has to be grayscale...

test/test_transforms_v2.py Outdated Show resolved Hide resolved
@ahmadsharif1
Copy link
Contributor Author

Thanks a lot @ahmadsharif1 , just one minor comment below but this looks good. Maybe you can undraft the PR? Now it's time to bikeshed about the name. I'm tempted to just call that ToRGB() instead of GrayScaleToRGB() so that it can be called at the beginning of every pipeline without suggesting that the input has to be grayscale...

I can change it to ToRGB, but while we are bikeshedding, I do like function names that start with a verb or action. So how about ConvertToRGB or MaybeExpandChannelsToRGB or something equivalent? WDYT?

I am happy to just go with ToRGB as well. Just let me know.

@ahmadsharif1 ahmadsharif1 marked this pull request as ready for review March 4, 2024 20:31
@NicolasHug
Copy link
Member

As discussed offline, we should just try to be consistent with the existing transforms here (even though the existing transforms themselves are not completely consistent...).

  • We already have the Grayscale transform, so I guess that means we should just have RGB()
  • We have the rgb_to_grayscale() functional, so lets' have grayscale_to_rgb() as well. (We do have to_grayscale(), but it's an oddity that only supports PIL images for whatever reason... so we shouldn't build on it)

Also before merging we'll have to add the transform and the functional to the docs here and here. To check for docs rendering you can either check out instructions in the contributing guide, or (probably easier for now) just wait for the CI job to be done and then check from #8247 (comment)

@ahmadsharif1 ahmadsharif1 merged commit 2bababf into pytorch:main Mar 15, 2024
74 of 75 checks passed
@ahmadsharif1 ahmadsharif1 deleted the i8186 branch March 15, 2024 16:52
Copy link

Hey @ahmadsharif1!

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

facebook-github-bot pushed a commit that referenced this pull request Mar 21, 2024
…#8247)

Reviewed By: vmoens

Differential Revision: D55062789

fbshipit-source-id: f98564a1276abccf2a0a126c6fa8846b93652cf6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

3 participants