-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Port test resize in test_transforms to pytest #3952
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AnirudhDagar , this looks great overall! I think we can make the tests more efficient with some parametrization, please see my comments below
Thanks a lot @NicolasHug for the feedback, I'll make these changes and update the PR. Btw, if possible, could you please help me with #3963 (Not Related to this PR)? |
Sorry for the delay @NicolasHug. This is now ready for review. Please let me know if there are some changes you would like to suggest. Thanks a lot for being patient and guiding me through! :)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!! thanks a lot @AnirudhDagar
Sorry for the delay
No worries at all, there's no real rush :)
t = transforms.Resize(osize, max_size=max_size) | ||
result = t(img) | ||
|
||
msg = "{}, {} - {} - {}".format(height, width, osize, max_size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for info: we don't need these kind of error message anymore as they just print the compared values. Now that we're using pytest, we can just rely on pytest introspection. It's fine to leave as-is though ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! I'll keep this in mind with my future PRs.
Reviewed By: NicolasHug Differential Revision: D29027325 fbshipit-source-id: 934371047fc2baa2b61f17832f5b251f745b31dc
Refactor Group J as mentioned in #3945