Raised value error on incorrect na_option #22037
Conversation
WillAyd left a comment
WillAyd left a comment There was a problem hiding this comment.
Thanks for the PR. Just need test(s) to ensure the behavior. Can also add a whatsnew for 0.24
| Also can you make sure this works for all of |
Codecov Report
@@ Coverage Diff @@ ## master #22037 +/- ## ========================================== + Coverage 92% 92% +<.01% ========================================== Files 170 170 Lines 50553 50556 +3 ========================================== + Hits 46510 46513 +3 Misses 4043 4043
Continue to review full report at Codecov.
|
| Not sure why travis-ci is failing. Looks unrelated/might have to do with an upstream dependency? Do I have to create a new PR or can I re-run the build somehow? |
WillAyd left a comment
WillAyd left a comment There was a problem hiding this comment.
One comment (applicable to all tests) but otherwise LGTM. Travis failures look unrelated so will see on re-push if resolved, but otherwise I don't think it's a blocker for this PR
pandas/tests/frame/test_rank.py Outdated
| with pytest.raises(ValueError) as excinfo: | ||
| self.frame.rank(na_option='bad', ascending=False) | ||
| | ||
| assert msg == str(excinfo.value) |
There was a problem hiding this comment.
Instead of doing this you can pass the expected msg as an argument to tm.assert_raises_regex
raguiar2 force-pushed the ra--add-value-error-to-rank branch from 9b8c978 to 59c98a6 Compare | @WillAyd PTAL! |
| lgtm do we have an open issue for this? |
| Updated original comment to make linkage clearer. Merging later today if no objections |
| Thanks @raguiar2! |
* added value error on incorrect na_option * added test cases
4 participants
git diff upstream/master -u -- "*.py" | flake8 --diff