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

RF: use isinstance instead of type().__name__ comparison #150

Merged
merged 1 commit into from Mar 12, 2019

Conversation

Projects
None yet
3 participants
@yarikoptic
Copy link
Member

yarikoptic commented Mar 12, 2019

ModuleNotFoundError is a subclass of ImportError:
https://docs.python.org/3/library/exceptions.html#exception-hierarchy
so we can safely check via isinstance. Moreover, it seems that
whenever exception is raised without explicit instantiation
(raise ImportError), we do catch an instance, so should work fine.

And I found not explicit comment or commit description on why I used type().__name__ here, may be just some paranoia

Thanks @effigies for the review/comment at #149 (comment)

RF: use isinstance instead of type().__name__ comparison
ModuleNotFoundError is a subclass of ImportError:
  https://docs.python.org/3/library/exceptions.html#exception-hierarchy
so we can safely check via isinstance.  Moreover, it seems that
whenever exception is raised without explicit instantiation
(raise ImportError), we do catch an instance, so should work fine
@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 12, 2019

Coverage Status

Coverage increased (+0.5%) to 86.345% when pulling b61da12 on yarikoptic:rf-isinstance into b053eeb on duecredit:master.

2 similar comments
@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 12, 2019

Coverage Status

Coverage increased (+0.5%) to 86.345% when pulling b61da12 on yarikoptic:rf-isinstance into b053eeb on duecredit:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 12, 2019

Coverage Status

Coverage increased (+0.5%) to 86.345% when pulling b61da12 on yarikoptic:rf-isinstance into b053eeb on duecredit:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 12, 2019

Codecov Report

Merging #150 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #150   +/-   ##
=======================================
  Coverage   83.22%   83.22%           
=======================================
  Files          45       45           
  Lines        2373     2373           
  Branches      259      259           
=======================================
  Hits         1975     1975           
  Misses        325      325           
  Partials       73       73
Impacted Files Coverage Δ
duecredit/stub.py 72% <50%> (ø) ⬆️

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 b053eeb...b61da12. Read the comment docs.

@yarikoptic yarikoptic merged commit 1c736a1 into duecredit:master Mar 12, 2019

4 of 5 checks passed

codecov/patch 50% of diff hit (target 83.22%)
Details
codecov/project 83.22% remains the same compared to b053eeb
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.5%) to 86.345%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.