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.

Sign up for GitHub

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

Jump to bottom

[pytest port] test{crop_five, crop_ten, resize, resized_crop} in test_transforms_tensor #4010

Merged
merged 12 commits into from
Jun 9, 2021
Merged

[pytest port] test{crop_five, crop_ten, resize, resized_crop} in test_transforms_tensor #4010

merged 12 commits into from
Jun 9, 2021

Conversation

AnirudhDagar
Copy link
Contributor

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

  • test_x_crop : This merges _test_op_list_output, test_five_crop, test_ten_crop by parametrizing over func, method, out_length and fn_kwargs.
  • test_resize
  • test_resized_crop

Ps. @NicolasHug I kept this PR separate from #4008 to make the reviews manageable. Please don't worry about merge conflicts. I'll fix those as soon as the other PR is merged.

AnirudhDagar
AnirudhDagar commented Jun 8, 2021
View reviewed changes
Copy link
Contributor Author

@AnirudhDagar AnirudhDagar left a comment

Choose a reason for hiding this comment

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

I've made a couple of comments. Since the code has been heavily refactored here, please feel free to ask any questions or doubts for clarification.

test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Show resolved Hide resolved
test/test_transforms_tensor.py Show resolved Hide resolved
@AnirudhDagar
Copy link
Contributor Author

Again the failing python_type_check is not related to this PR. I can send a fix for them after a discussion on #4009.

@AnirudhDagar AnirudhDagar mentioned this pull request Jun 8, 2021
Closed
@datumbox datumbox mentioned this pull request Jun 9, 2021
Merged
@NicolasHug NicolasHug mentioned this pull request Jun 9, 2021
Closed
6 tasks
NicolasHug
NicolasHug reviewed Jun 9, 2021
View reviewed changes
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
AnirudhDagar
AnirudhDagar commented Jun 9, 2021
View reviewed changes
Copy link
Contributor Author

@AnirudhDagar AnirudhDagar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This should be ready now. Thanks a tonne @datumbox @NicolasHug for all the help.

If everything goes green and if this looks good to you, I'll fix the merge conflicts with master and update the branch for a merge. As a last thing I'll just club all the resize tests into a single class.

test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
NicolasHug
NicolasHug reviewed Jun 9, 2021
View reviewed changes
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.

Great work @AnirudhDagar , thanks a lot!
I made a few comments but this looks good

test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
test/test_transforms_tensor.py Outdated Show resolved Hide resolved
NicolasHug
NicolasHug approved these changes Jun 9, 2021
View reviewed changes
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 @AnirudhDagar ! I'll merge when green

@NicolasHug
Copy link
Member

Looks like binary_win_conda_py3.9_cu111 will timeout again, all unittests workflows are passing so it's good to merge

@NicolasHug NicolasHug merged commit 810811f into pytorch:master Jun 9, 2021
@AnirudhDagar AnirudhDagar deleted the refactor-test-tensor-transforms-x-crop-resize branch June 9, 2021 12:37
@AnirudhDagar
Copy link
Contributor Author

Thanks again for the detailed reviews and guidance. Enjoyed this! :))

@NicolasHug NicolasHug mentioned this pull request Jun 9, 2021
Merged
facebook-github-bot pushed a commit that referenced this pull request Jun 14, 2021
@NicolasHug @facebook-github-bot
[fbsync] [pytest port] test{crop_five, crop_ten, resize, resized_crop…
4516390 
…} in test_transforms_tensor (#4010)

Reviewed By: fmassa

Differential Revision: D29097743

fbshipit-source-id: 0c8900d69c8092abfdf00d6abd3bee6c8db1d8e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datumbox datumbox datumbox left review comments

@NicolasHug NicolasHug NicolasHug approved these changes

Assignees
No one assigned
Labels
cla signed code quality module: tests module: transforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AnirudhDagar @NicolasHug @datumbox @facebook-github-bot