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

MAINT: Remove self.assertFalse from testing #16151

Merged
merged 1 commit into from
Apr 27, 2017

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Apr 26, 2017

Title is self-explanatory.

Partially addresses #15990.

@gfyoung gfyoung changed the title MAINT: self.assertFalse from testing MAINT: Remove self.assertFalse from testing Apr 26, 2017
@jorisvandenbossche jorisvandenbossche added Clean Testing pandas testing functions or related to the test suite labels Apr 26, 2017
@jorisvandenbossche jorisvandenbossche added this to the 0.20.0 milestone Apr 26, 2017
@jorisvandenbossche
Copy link
Member

One remark for this replacement: in certain cases it might be that we want to test that something is actually False, and not falsey (for which you would need to do assert xx is False instead of assert not xx)
But, this is difficult to say without looking in more detail into each of the tests, which is a bit too much work ..

@gfyoung
Copy link
Member Author

gfyoung commented Apr 26, 2017

@jorisvandenbossche : Well according to unittest documentation, assertFalse(expr) checks that bool(expr) is False, so I think that concern might be moot.

@jorisvandenbossche
Copy link
Member

Ah, you're correct. The implementation actually just does if expr: raise .., which is equivalent.

@max-sixty
Copy link
Contributor

@gfyoung you're a hero for running through all these!

@gfyoung
Copy link
Member Author

gfyoung commented Apr 26, 2017

@MaximilianR : Thanks! Couldn't do it without bash ;)

@codecov
Copy link

codecov bot commented Apr 26, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16151   +/-   ##
=======================================
  Coverage   90.83%   90.83%           
=======================================
  Files         159      159           
  Lines       50796    50796           
=======================================
  Hits        46143    46143           
  Misses       4653     4653
Flag Coverage Δ
#multiple 88.62% <ø> (ø) ⬆️
#single 40.31% <ø> (ø) ⬆️

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 3b80ed3...ecc0590. Read the comment docs.

@jreback jreback merged commit cefc8c0 into pandas-dev:master Apr 27, 2017
@jreback
Copy link
Contributor

jreback commented Apr 27, 2017

thanks!

@gfyoung gfyoung deleted the assert-false-remove branch April 27, 2017 00:43
pcluo pushed a commit to pcluo/pandas that referenced this pull request May 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants