Skip to content

[proto] Use the proper _transformed_types in all Transforms and eliminate unnecessary dispatching #6494

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 15 commits into from
Aug 25, 2022

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Aug 25, 2022

Addresses some of the remarks at #6486

This PR:

  • Adds the appropriate _transformed_types types to all transforms
  • Removes unnecessary dispatching within the _transform()
  • Specifies where possible the return types of _transform()

# Conflicts:
#	torchvision/prototype/transforms/_deprecated.py
#	torchvision/prototype/transforms/_type_conversion.py
@datumbox datumbox marked this pull request as draft August 25, 2022 11:42
@datumbox datumbox changed the title [WIP] Fix transformed types Use the proper _transformed_types in all Transforms and eliminate unnecessary dispatching Aug 25, 2022
@datumbox datumbox requested review from vfdev-5 and pmeier August 25, 2022 11:52
@datumbox datumbox marked this pull request as ready for review August 25, 2022 11:52
@datumbox datumbox changed the title Use the proper _transformed_types in all Transforms and eliminate unnecessary dispatching [proto] Use the proper _transformed_types in all Transforms and eliminate unnecessary dispatching Aug 25, 2022
@datumbox datumbox force-pushed the fix_transformed_types branch from 323aed2 to 79944f6 Compare August 25, 2022 12:01
Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

One question. Otherwise LGTM.

@datumbox datumbox requested a review from pmeier August 25, 2022 12:33
Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

LGTM!

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Aug 25, 2022

@datumbox CI failures are related : https://github.com/pytorch/vision/runs/8015985956?check_suite_focus=true

=================================== FAILURES ===================================
__________________ TestToImageTensor.test__transform[Image1] ___________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms.py", line 1048, in test__transform
    fn.assert_called_once_with(inpt, copy=transform.copy)
  File "/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/unittest/mock.py", line 888, in assert_called_once_with
    raise AssertionError(msg)
AssertionError: Expected 'to_image_tensor' to be called once. Called 0 times.
____________________ TestToImagePIL.test__transform[Image0] ____________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms.py", line 1065, in test__transform
    fn.assert_called_once_with(inpt, mode=transform.mode)
  File "/opt/hostedtoolcache/Python/3.7.13/x64/lib/python3.7/unittest/mock.py", line 888, in assert_called_once_with
    raise AssertionError(msg)
AssertionError: Expected 'to_image_pil' to be called once. Called 0 times.
________________________ test_to_image_pil[None-inpt2] _________________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1907, in test_to_image_pil
    output = F.to_image_pil(inpt, mode=mode)
  File "/home/runner/work/vision/vision/torchvision/transforms/functional.py", line 259, in to_pil_image
    raise TypeError(f"pic should be Tensor or ndarray. Got {type(pic)}.")
TypeError: pic should be Tensor or ndarray. Got <class 'PIL.Image.Image'>.
_________________________ test_to_image_pil[RGB-inpt2] _________________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1907, in test_to_image_pil
    output = F.to_image_pil(inpt, mode=mode)
  File "/home/runner/work/vision/vision/torchvision/transforms/functional.py", line 259, in to_pil_image
    raise TypeError(f"pic should be Tensor or ndarray. Got {type(pic)}.")
TypeError: pic should be Tensor or ndarray. Got <class 'PIL.Image.Image'>.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Aug 25, 2022

@datumbox CI failures are still related to your latest commit:

=================================== FAILURES ===================================
_______________________ test_to_image_tensor[True-inpt0] _______________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1877, in test_to_image_tensor
    output = F.to_image_tensor(inpt, copy=copy)
TypeError: to_image_tensor() got an unexpected keyword argument 'copy'
_______________________ test_to_image_tensor[True-inpt1] _______________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1877, in test_to_image_tensor
    output = F.to_image_tensor(inpt, copy=copy)
TypeError: to_image_tensor() got an unexpected keyword argument 'copy'
_______________________ test_to_image_tensor[True-inpt2] _______________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1877, in test_to_image_tensor
    output = F.to_image_tensor(inpt, copy=copy)
TypeError: to_image_tensor() got an unexpected keyword argument 'copy'
______________________ test_to_image_tensor[False-inpt0] _______________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1877, in test_to_image_tensor
    output = F.to_image_tensor(inpt, copy=copy)
TypeError: to_image_tensor() got an unexpected keyword argument 'copy'
______________________ test_to_image_tensor[False-inpt1] _______________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1877, in test_to_image_tensor
    output = F.to_image_tensor(inpt, copy=copy)
TypeError: to_image_tensor() got an unexpected keyword argument 'copy'
______________________ test_to_image_tensor[False-inpt2] _______________________
Traceback (most recent call last):
  File "/home/runner/work/vision/vision/test/test_prototype_transforms_functional.py", line 1877, in test_to_image_tensor
    output = F.to_image_tensor(inpt, copy=copy)
TypeError: to_image_tensor() got an unexpected keyword argument 'copy'

You can check that locally: pytest -vvv test/test_prototype_transforms_functional.py

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks @datumbox !

@datumbox datumbox merged commit 9f6a189 into pytorch:main Aug 25, 2022
@datumbox datumbox deleted the fix_transformed_types branch August 25, 2022 15:44
facebook-github-bot pushed a commit that referenced this pull request Aug 30, 2022
…s and eliminate unnecessary dispatching (#6494)

Summary:
* Update types in deprecated transforms.

* Update types in type conversion transforms.

* Fixing types in meta transforms.

* More changes on type conversion.

* Bug fix.

* Fix types

* Remove unnecessary conversions.

* Remove unnecessary import.

* Fixing tests

* Remove copy support from `to_image_tensor`

* restore test param

* Fix further tests

Reviewed By: NicolasHug

Differential Revision: D39131008

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

4 participants