Skip to content

Simplify TestCase.assertExpected() #2815

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 6 commits into from
Oct 14, 2020
Merged

Conversation

datumbox
Copy link
Contributor

Minor Refactoring on TestCase.assertExpected() to fix #2814

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.

Looks good ! Thanks @datumbox !

@datumbox datumbox requested a review from vfdev-5 October 14, 2020 17:03
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.

LGTM!

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Oct 14, 2020

Seems like the CI is a bit confused by the last commit as circle ci tests are not shown...

@fmassa
Copy link
Member

fmassa commented Oct 14, 2020

Same thing is happening on #2812, maybe it's a CircleCI issue?

EDIT: Seems to be working now, I'll rebase this on top of master once the other PR gets merged to kick CI again here

@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #2815 into master will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2815      +/-   ##
==========================================
+ Coverage   73.32%   73.43%   +0.11%     
==========================================
  Files          99       99              
  Lines        8726     8771      +45     
  Branches     1374     1376       +2     
==========================================
+ Hits         6398     6441      +43     
- Misses       1909     1910       +1     
- Partials      419      420       +1     
Impacted Files Coverage Δ
torchvision/transforms/functional_tensor.py 75.93% <0.00%> (+2.02%) ⬆️

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 69dc971...bb056a8. Read the comment docs.

Copy link
Member

@fmassa fmassa 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 !

@fmassa fmassa merged commit 16ef312 into pytorch:master Oct 14, 2020
@datumbox datumbox deleted the bugfix/issue2814 branch October 14, 2020 19:43
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
* Removing unnecessary variable.

* Refactoring common_utils.py to remove unused vars.

* Removing unused value and changing use of accept_output.

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
vfdev-5 added a commit to Quansight/vision that referenced this pull request Dec 4, 2020
* Removing unnecessary variable.

* Refactoring common_utils.py to remove unused vars.

* Removing unused value and changing use of accept_output.

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify logic on TestCase.assertExpected()
3 participants