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.

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

Cleanup functional_tensor.py (#3159) #3171

Merged
merged 13 commits into from
Dec 15, 2020
Merged

Cleanup functional_tensor.py (#3159) #3171

merged 13 commits into from
Dec 15, 2020

Conversation

avijit9
Copy link
Contributor

@avijit9 avijit9 commented Dec 14, 2020

Fixes #3159

@facebook-github-bot
Copy link

Hi @avijit9!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@datumbox datumbox self-requested a review December 14, 2020 12:26
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

@avijit9 Thanks a lot for the PR and for your investigation on the original proposal!

I left a few comments, please let me know what you think.

test/test_functional_tensor_cleanup.py Outdated Show resolved Hide resolved
test/test_functional_tensor_cleanup.py Outdated Show resolved Hide resolved
test/test_functional_tensor_cleanup.py Outdated Show resolved Hide resolved
torchvision/transforms/functional_tensor.py Show resolved Hide resolved
torchvision/transforms/functional_tensor.py Show resolved Hide resolved
torchvision/transforms/functional_tensor.py Show resolved Hide resolved
torchvision/transforms/functional_tensor.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 14, 2020

Codecov Report

Merging #3171 (5ec23a9) into master (90645cc) will increase coverage by 0.39%.
The diff coverage is 78.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3171      +/-   ##
==========================================
+ Coverage   73.09%   73.48%   +0.39%     
==========================================
  Files          99       99              
  Lines        9241     9230      -11     
  Branches     1493     1476      -17     
==========================================
+ Hits         6755     6783      +28     
+ Misses       2015     1991      -24     
+ Partials      471      456      -15     
Impacted Files Coverage Δ
torchvision/transforms/functional_tensor.py 79.34% <78.78%> (+6.87%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90645cc...5ec23a9. Read the comment docs.

@avijit9 avijit9 requested a review from datumbox December 14, 2020 14:46
@datumbox
Copy link
Contributor

@avijit9 Thanks for the changes. Your PR looks great!

Note that we just merged a PR that adds a 6 more functional transforms. Would you mind making your changes to those as well?

if not _is_tensor_a_torch_image(img):
raise TypeError('tensor is not a torch image.')

if not _is_tensor_a_torch_image(img):
raise TypeError('tensor is not a torch image.')

if not _is_tensor_a_torch_image(img):
raise TypeError('tensor is not a torch image.')

if not _is_tensor_a_torch_image(img):
raise TypeError('tensor is not a torch image.')

if not _is_tensor_a_torch_image(img):
raise TypeError('tensor is not a torch image.')

if not _is_tensor_a_torch_image(img):
raise TypeError('tensor is not a torch image.')

If you are busy let me know and I'll do it. Just make sure I got write access to your branch and I'll push it there so that you get credit for the changes.

@avijit9
Copy link
Contributor Author

avijit9 commented Dec 15, 2020

@datumbox No problem. I have made the changes and pushing them now. Thanks for being awesome :)

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

@avijit9 Looks great! I added one comment which I think it's due to wrong copy-paste. Let me know if you agree. :)

test/test_functional_tensor.py Outdated Show resolved Hide resolved
@datumbox datumbox merged commit 1a300d8 into pytorch:master Dec 15, 2020
@avijit9 avijit9 deleted the cleanup_functional_tensor branch December 16, 2020 02:47
facebook-github-bot pushed a commit that referenced this pull request Dec 23, 2020
Summary:
* added the helper method for dimension checks

* unit tests for dimensio check function in functional_tensor

* code formatting and typing

* moved torch image check after tensor check

* unit testcases for test_assert_image_tensor added and refactored

* separate unit testcase file deleted

* assert_image_tensor added to newly created 6 methods

* test cases added for new 6 mthohds

* removed wrongly pasted posterize method and added solarize method for testing

Reviewed By: fmassa

Differential Revision: D25679214

fbshipit-source-id: 60ca5c1e6a653195a3dd07755b7ac7fa6d4eaf4b

Co-authored-by: Vasilis Vryniotis <[email protected]>
Co-authored-by: Vasilis Vryniotis <[email protected]>
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.

Simplify dimension checks on functional_tensor.py
3 participants