-
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 vertical flip #7712
port vertical flip #7712
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7712
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 3 Unrelated FailuresAs of commit 49341c0: NEW FAILURES - The following jobs have failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base 0e49615:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@@ -93,7 +93,8 @@ def vertical_flip_image_tensor(image: torch.Tensor) -> torch.Tensor: | |||
return image.flip(-2) | |||
|
|||
|
|||
vertical_flip_image_pil = _FP.vflip | |||
def vertical_flip_image_pil(image: PIL.Image) -> PIL.Image: |
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.
Same as #7703 (comment)
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 Philip, LGTM but did we consider a base class for both TestHorizontalFlip
and TestVerticalFlip
? They look like identical copies with only a few names / callable / matrices changes
They are. I didn't find a good pattern using base classes: class Base:
FOO = None
@pytest.mark.parametrize("foo", FOO)
def test_foo(self):
...
class Test(Base):
FOO = ["foo", "bar", "baz"] The snippet above does not work, since Another approach would be to merge both classes into one @pytest.mark.parametrize("kernel", [F.horizontal_flip_image_tensor, F.vertical_flip_image_tensor])
def test_image_kernel(kernel):
check_kernel(kernel, ...) If we want that, I'll implement it in a follow-up PR. |
Hey @pmeier! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
Reviewed By: vmoens Differential Revision: D47186566 fbshipit-source-id: 92dd32411629e98d4b82c69cd9a000bd92eeb5fb
cc @vfdev-5