Skip to content

Simplify the assertExpected method #2965

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 2 commits into from
Nov 5, 2020

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Nov 5, 2020

The assertExpected method is unnecessarily complicated. In this PR I separate the estimation of the expected filename from the main assertion and simplify the logic of accepting a new expected value.

@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #2965 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2965   +/-   ##
=======================================
  Coverage   73.43%   73.43%           
=======================================
  Files          99       99           
  Lines        8813     8813           
  Branches     1391     1391           
=======================================
  Hits         6472     6472           
  Misses       1916     1916           
  Partials      425      425           

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 32e5700...23c153c. Read the comment docs.

@datumbox datumbox requested a review from fmassa November 5, 2020 13:58
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 a lot for the cleanup!

I don't think we need this anymore, just putting some references there for some history.

@fmassa fmassa merged commit 4738267 into pytorch:master Nov 5, 2020
@datumbox datumbox deleted the refactor/simplify_assertExpected branch November 5, 2020 18:10
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
* Simplify the ACCEPT=True logic in assertExpected().

* Separate the expected filename estimation from assertExpected
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
* Simplify the ACCEPT=True logic in assertExpected().

* Separate the expected filename estimation from assertExpected
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.

3 participants