Skip to content

Conversation

AnirudhDagar
Copy link
Contributor

This PR ports group B from #3987. All the tests below run on cpu_and_gpu and have been parametrized accordingly.

  • test_color_jitter : split into five new test functions as below
    • test_color_jitter_brightness
    • test_color_jitter_contrast
    • test_color_jitter_saturation
    • test_color_jitter_hue
    • test_color_jitter_all
  • test_pad
  • test_crop : split into two test functions.
    • test_crop
    • test_crop_pad
  • test_center_crop : There was no need to split or parameterize this.



@pytest.mark.parametrize('device', cpu_and_gpu())
class TestColorJitter:
Copy link
Contributor Author

@AnirudhDagar AnirudhDagar Jun 8, 2021

Choose a reason for hiding this comment

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

Thought it would be better to have a separate class for all these tests. Also, avoids extra lines of code repetition of @pytest.mark.parametrize('device', cpu_and_gpu()) for each test.

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 @AnirudhDagar ! I'll sync with master and merge

Comment on lines +683 to +690
@pytest.mark.parametrize('padding_config', [
{"padding_mode": "constant", "fill": 0},
{"padding_mode": "constant", "fill": 10},
{"padding_mode": "constant", "fill": 20},
{"padding_mode": "edge"},
{"padding_mode": "reflect"}
])
@pytest.mark.parametrize('size', [5, [5, ], [6, 6]])
Copy link
Member

Choose a reason for hiding this comment

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

This parametrization and the config["size"] = size below is a bit unnatural. I think it mostly comes from the fact that we want to avoid passing a default for fill but its default is 0 and we already hardcode it here. Maybe parametrizing over 'padding_mode, fill' and 'size' would make more sense, instead of parametrizing over 'padding_config'.

But it's not super important, so let's leave it as is!

@NicolasHug NicolasHug merged commit 69f2e1b into pytorch:master Jun 9, 2021
facebook-github-bot pushed a commit that referenced this pull request Jun 14, 2021
…ransforms_tensor (#4008)

Reviewed By: fmassa

Differential Revision: D29097726

fbshipit-source-id: 4fe118454563828f6413804f9bcee7c5b2fa74f8
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.

3 participants