Skip to content

transforms: randomly grayscaling an image #325

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

Merged
merged 5 commits into from
Nov 13, 2017
Merged

transforms: randomly grayscaling an image #325

merged 5 commits into from
Nov 13, 2017

Conversation

sourabhd
Copy link
Contributor

@sourabhd sourabhd commented Nov 7, 2017

Added transform for data augmetation technique where an image is grayscaled with probability p. The grayscaled version of image can be three channel (r == g == b) or single channel.

For detailed discussion of this feature please refer to issue #299

Copy link
Contributor

@alykhantejani alykhantejani left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add some simple tests to check that to_grayscale returns the expected number of channels and data?

Copy link
Contributor

@alykhantejani alykhantejani left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add some simple tests to check that to_grayscale returns the expected number of channels and data?

@sourabhd
Copy link
Contributor Author

sourabhd commented Nov 9, 2017

@alykhantejani

  1. I have added test cases
  2. Subtle change to the interface:
    F.to_grayscale() remains same. We have two transforms:
  • Grayscale(num_output_channels=1) : converts RGB/1-channel grayscale to 1 channel or 3 channel grayscale as desired
  • RandomGrayscale(p=0.1) : randomly grayscales image, but retains the number of channels. [Does not make sense to have have few 3 channel and few 1 channel images in a batch]
    Please review the changes again

@alykhantejani
Copy link
Contributor

alykhantejani commented Nov 9, 2017

@sourabhd Thanks for making the changes. There seems to be some merge conflicts with master that need to be resolved before we can merge

@sourabhd
Copy link
Contributor Author

sourabhd commented Nov 9, 2017

@alykhantejani RandomRotation and RandomGrayscale seem to be added at the same time in same files. Not a real conflict - just need to move lines of code and it should be fine. Write access is needed for merge, which I do not have.

@alykhantejani
Copy link
Contributor

alykhantejani commented Nov 9, 2017

@sourabhd yup, I can do the merge once the conflict is resolved. Can you do this locally and push to origin? i.e.:

git remote add upstream [email protected]:pytorch/vision.git
git pull upstream master
<this will cause a merge conflict>
<fix conflict>
git commit -am "merge upstream"
git push origin master

np.testing.assert_equal(gray_np_4[:, :, 1], gray_np_4[:, :, 2])
np.testing.assert_equal(gray_np, gray_np_4[:, :, 0])

# Set 2: Randomly grayscale image

This comment was marked as off-topic.


"""

def __init__(self, p=0.1):

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants