BUG: Add type check for width parameter in str.pad method GH13598 #13690
Conversation
|
Thx! can u add tests all methods uses |
| @@ -614,3 +614,4 @@ Bug Fixes | |||
| - Bug in ``groupby`` with ``as_index=False`` returns all NaN's when grouping on multiple columns including a categorical one (:issue:`13204`) | |||
|
|
|||
| - Bug where ``pd.read_gbq()`` could throw ``ImportError: No module named discovery`` as a result of a naming conflict with another python package called apiclient (:issue:`13454`) | |||
| - Bug in ``pandas.Series.str.zfill``, no TypeErrors raised when ``width`` was not of integer type (:issue:`13598`) | |||
There was a problem hiding this comment.
nice to describe the results are all NaN. Also, TypeError
|
I wrote all the tests, however it seemed extremely redundant doing so. I know that redundancy isn't necessarily a bad thing when testing, but is there a better way to implement them? Thanks |
| @@ -1603,6 +1603,18 @@ def test_pad_fillchar(self): | |||
| "fillchar must be a character, not int"): | |||
| result = values.str.pad(5, fillchar=5) | |||
|
|
|||
| def test_pad_width(self): | |||
There was a problem hiding this comment.
you can just put them all in a single test method and iterate over the functions asserting that they raise
There was a problem hiding this comment.
@jreback I'm relatively new to OOP in python, so could you let me know if there's anything I could do to improve my current implementation?
Thanks
Current coverage is 84.38%@@ master #13690 diff @@
==========================================
Files 142 142
Lines 51223 51226 +3
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 43224 43227 +3
Misses 7999 7999
Partials 0 0
|
… raise correctly."
| @@ -914,6 +914,10 @@ def str_pad(arr, width, side='left', fillchar=' '): | |||
| if len(fillchar) != 1: | |||
| raise TypeError('fillchar must be a character, not str') | |||
|
|
|||
| if not isinstance(width, compat.integer_types): | |||
| msg = 'width must be of integer type, not {0}' | |||
|
thanks @wcwagner |
git diff upstream/master | flake8 --diff