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

CLN: Enforce E721, use isinstance #4925

Merged
merged 1 commit into from
Sep 12, 2018
Merged

Conversation

bashtage
Copy link
Member

Use isinstance instead of comparing types

@josef-pkt
Copy link
Member

in some cases checking 'type' explicitly is intentional, when we don't want to allow for subclasses.
e.g. type(x) is ndarray is not true if x is a subclass of numpy array.

I guess in unit tests it's better to require a specific class/type, instead of just being and instance of a class or subclass of it.

@bashtage bashtage force-pushed the flake8-e721 branch 3 times, most recently from f782a9c to b6b4069 Compare August 20, 2018 20:46
@coveralls
Copy link

coveralls commented Aug 21, 2018

Coverage Status

Coverage increased (+0.007%) to 83.926% when pulling 4478466 on bashtage:flake8-e721 into 35ae4c3 on statsmodels:master.

@codecov-io
Copy link

codecov-io commented Aug 21, 2018

Codecov Report

Merging #4925 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4925      +/-   ##
==========================================
+ Coverage   81.41%   81.41%   +<.01%     
==========================================
  Files         581      581              
  Lines       90278    90279       +1     
  Branches    10101    10101              
==========================================
+ Hits        73504    73505       +1     
  Misses      14485    14485              
  Partials     2289     2289
Impacted Files Coverage Δ
statsmodels/tsa/tests/test_arima.py 99.33% <100%> (ø) ⬆️
statsmodels/base/tests/test_shrink_pickle.py 97.71% <100%> (ø) ⬆️
statsmodels/tsa/base/tests/test_tsa_indexes.py 95.66% <100%> (ø) ⬆️
statsmodels/stats/mediation.py 94.93% <100%> (+0.03%) ⬆️
statsmodels/tsa/tests/test_ar.py 100% <100%> (ø) ⬆️
statsmodels/tsa/vector_ar/tests/test_var.py 93.51% <100%> (ø) ⬆️

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 35ae4c3...4478466. Read the comment docs.

@bashtage bashtage force-pushed the flake8-e721 branch 3 times, most recently from 8cf08ac to e44fe02 Compare August 28, 2018 07:32
@bashtage bashtage force-pushed the flake8-e721 branch 3 times, most recently from b144cd4 to 83177ac Compare September 9, 2018 07:40
@jbrockmendel jbrockmendel mentioned this pull request Sep 9, 2018
32 tasks
@@ -111,7 +111,7 @@ def test_pickle_wrapper(self):
res_unpickled = self.results.__class__.load(fh)
fh.close()
# print type(res_unpickled)
assert_(type(res_unpickled) is type(self.results))
assert_(type(res_unpickled) is type(self.results)) # noqa
Copy link
Contributor

Choose a reason for hiding this comment

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

Print above unnecessary. Worth changing to plain asserts?

Use isinstance instead of comparing types
@josef-pkt
Copy link
Member

as green as it gets. merging

@bashtage Thank you

@josef-pkt josef-pkt merged commit c2f6b08 into statsmodels:master Sep 12, 2018
@bashtage bashtage deleted the flake8-e721 branch September 12, 2018 17:13
@josef-pkt josef-pkt added this to the 0.10 milestone Sep 15, 2018
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.

5 participants