Skip to content

RGB to Grayscale #1772

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

Closed

Conversation

ghosalsattam
Copy link
Contributor

@ghosalsattam ghosalsattam commented May 2, 2020

See #1333 #1353 #1226
This PR is responsible for RGB to Grayscale Conversion of image(s).

@ghosalsattam
Copy link
Contributor Author

@bhack @gabrieldemarmiesse @seanpmorgan @WindQAQ Please provide a review.

Copy link
Member

@WindQAQ WindQAQ left a comment

Choose a reason for hiding this comment

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

Personally, I don't think we should have this as it is merely a single-lined wrapper of tf.image.rgb_to_grayscale.

tf.image.rgb_to_grayscale has the ability to convert image with shape [H, W, 3] and [B, H, W, 3]. For the 2D image, the function in this PR is going to convert [H, W] to [1, H, W, 1] and raise the exception. It turns out this that function can only do the same functionality as tf.image.rgb_to_grayscale does.

https://colab.research.google.com/drive/1v7C-DkHYJijgIdrwKhMpXmVqrJbLW8Sf

@tensorflow/sig-addons-maintainers Hi all, any thought on this? Thanks.

@gabrieldemarmiesse
Copy link
Member

I agree with @WindQAQ , there is not enough added value here compared to what tensorflow offers, we can use directly tf.image.rgb_to_grayscale . I think we need to edit #1353 (comment) . I can understand why someone might think that we need to copy the function here. I'll correct the message.

Thank you @ghosalsattam for the pull request, and sorry for not looking too much into the issue. I'll check the box for rgb to grayscale.

@bhack
Copy link
Contributor

bhack commented May 2, 2020

I agree cause in our cause just for this single line call we need +137 lines of diff (overhead, tests etc.) also if the original wrapper was just 5 lines.

@ghosalsattam ghosalsattam deleted the rgb_to_grayscale branch May 11, 2020 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants