Skip to content

MAINT: Strip internals from TestCase class #16016

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 4 commits into from
Apr 16, 2017

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Apr 16, 2017

TestCase is a vestige from the nosetest framework. We want to discourage usage of self.* method calls under our new pytest framework, so this PR strips as much as possible from the class.

Partially addresses #15990.

@gfyoung gfyoung force-pushed the pytest-idiom-convert branch from 6e7b193 to e79aaa7 Compare April 16, 2017 04:20
@codecov
Copy link

codecov bot commented Apr 16, 2017

Codecov Report

Merging #16016 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16016      +/-   ##
==========================================
- Coverage      91%   90.99%   -0.01%     
==========================================
  Files         153      153              
  Lines       50481    50469      -12     
==========================================
- Hits        45938    45926      -12     
  Misses       4543     4543
Flag Coverage Δ
#multiple 88.76% <100%> (-0.01%) ⬇️
#single 40.44% <66.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/util/decorators.py 65.3% <ø> (ø) ⬆️
pandas/util/testing.py 80.8% <100%> (-0.21%) ⬇️

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 3119e90...e79aaa7. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 16, 2017

Codecov Report

Merging #16016 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16016      +/-   ##
==========================================
- Coverage   90.99%   90.99%   -0.01%     
==========================================
  Files         154      154              
  Lines       50484    50471      -13     
==========================================
- Hits        45940    45928      -12     
+ Misses       4544     4543       -1
Flag Coverage Δ
#multiple 88.76% <100%> (-0.01%) ⬇️
#single 40.44% <66.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/util/decorators.py 65.3% <ø> (ø) ⬆️
pandas/util/testing.py 80.78% <100%> (-0.23%) ⬇️
pandas/core/common.py 91.03% <0%> (+0.34%) ⬆️

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 8daf9a7...b2b6802. Read the comment docs.

@jorisvandenbossche
Copy link
Member

An alternative would be to keep it as class methods, but remove the unittest.TestCase inheritance (just pointing out, I am personally OK with moving from self. to tm.)

@jreback jreback added the Testing pandas testing functions or related to the test suite label Apr 16, 2017
@jreback
Copy link
Contributor

jreback commented Apr 16, 2017

can you rebase.

lgtm otherwise.

@jreback
Copy link
Contributor

jreback commented Apr 16, 2017

An alternative would be to keep it as class methods, but remove the unittest.TestCase inheritance (just pointing out, I am personally OK with moving from self. to tm.)

this is a precursor step to this. That's the goal.

@jreback jreback added this to the 0.20.0 milestone Apr 16, 2017
@jorisvandenbossche
Copy link
Member

this is a precursor step to this. That's the goal.

No, I mean that, if TestCase no longer inherits from unittest.TestCase, you can keep using self., so what I mean is that this is not a necessary precursor towards that goal

@gfyoung gfyoung force-pushed the pytest-idiom-convert branch from e79aaa7 to b2b6802 Compare April 16, 2017 16:25
@gfyoung
Copy link
Member Author

gfyoung commented Apr 16, 2017

@jorisvandenbossche : Perhaps, but why should we though? These methods were never really implemented as instance methods in the first place.

@jreback
Copy link
Contributor

jreback commented Apr 16, 2017

yeah I think we should move aways from using self entirely for actually testing things (keep state is fine).

@gfyoung
Copy link
Member Author

gfyoung commented Apr 16, 2017

@jreback , @jorisvandenbossche : Rebased, all green, and ready to merge.

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

jreback commented Apr 16, 2017

thanks!

@gfyoung gfyoung deleted the pytest-idiom-convert branch April 16, 2017 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants