Skip to content

REF: Convert test_hashing to pytest idiom #24009

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 30, 2018
Merged

REF: Convert test_hashing to pytest idiom #24009

merged 2 commits into from
Nov 30, 2018

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Nov 30, 2018

No description provided.

@gfyoung gfyoung added Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite labels Nov 30, 2018
@gfyoung gfyoung added this to the 0.24.0 milestone Nov 30, 2018
@pep8speaks
Copy link

pep8speaks commented Nov 30, 2018 •
edited
Loading

Hello @gfyoung! Thanks for updating the PR.

Comment last updated on November 30, 2018 at 06:21 Hours UTC

@gfyoung gfyoung force-pushed the test-hashing-to-pytest branch from f1bbe88 to 77f7ac8 Compare November 30, 2018 06:21
@codecov
Copy link

codecov bot commented Nov 30, 2018 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #24009   +/-   ##
=======================================
  Coverage   92.31%   92.31%           
=======================================
  Files         161      161           
  Lines       51549    51549           
=======================================
  Hits        47586    47586           
  Misses       3963     3963
Flag Coverage Δ
#multiple 90.7% <ø> (ø) ⬆️
#single 42.44% <ø> (ø) ⬆️

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 dc8d35a...dfa24a8. Read the comment docs.

@gfyoung
BUG: Actually check index=True/False in some cases
dfa24a8 
_check_equal appeared to be checking index=True/False,
but we actually need to do explicitly pass in
index=True/False for this to work.
@gfyoung
Copy link
Member Author

gfyoung commented Nov 30, 2018

@jreback @TomAugspurger : Addressed the comment above, and all is still green. PTAL.

@TomAugspurger
Copy link
Contributor

LGTM. Instead of the index fixture you could also just call each of index=False and index=True in _check_equal, but either way works for me.

@gfyoung
Copy link
Member Author

gfyoung commented Nov 30, 2018 •
edited
Loading

Instead of the index fixture you could also just call each of index=False and index=True in _check_equal, but either way works for me.

Thought about it. Just wanted something a little more modular. Thanks for noticing the bug though!

@gfyoung gfyoung merged commit d1a2f5f into pandas-dev:master Nov 30, 2018
@gfyoung gfyoung deleted the test-hashing-to-pytest branch November 30, 2018 20:46
saurav2608 pushed a commit to saurav2608/pandas that referenced this pull request Dec 1, 2018
@gfyoung @saurav-chakravorty
REF: Convert test_hashing to pytest idiom (pandas-dev#24009)
5cf8ff7 
Also patch bug in the tests as follows:

_check_equal appeared to be checking index=True/False,
but we actually need to do explicitly pass in
index=True/False for this to work.
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@gfyoung @Pingviinituutti
REF: Convert test_hashing to pytest idiom (pandas-dev#24009)
735f3cf 
Also patch bug in the tests as follows:

_check_equal appeared to be checking index=True/False,
but we actually need to do explicitly pass in
index=True/False for this to work.
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@gfyoung @Pingviinituutti
REF: Convert test_hashing to pytest idiom (pandas-dev#24009)
1710d82 
Also patch bug in the tests as follows:

_check_equal appeared to be checking index=True/False,
but we actually need to do explicitly pass in
index=True/False for this to work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAugspurger TomAugspurger TomAugspurger approved these changes

Assignees
No one assigned
Labels
Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.
3 participants
@gfyoung @pep8speaks @TomAugspurger