From 468df3abfc7454504bfcbbd341a6e405ddcbb2d2 Mon Sep 17 00:00:00 2001 From: wcwagner Date: Sun, 17 Jul 2016 14:58:35 -0400 Subject: [PATCH 1/4] BUG: Add type check for width parameter in str.pad method GH13598 --- doc/source/whatsnew/v0.19.0.txt | 1 + pandas/core/strings.py | 4 ++++ pandas/tests/test_strings.py | 12 ++++++++++++ 3 files changed, 17 insertions(+) diff --git a/doc/source/whatsnew/v0.19.0.txt b/doc/source/whatsnew/v0.19.0.txt index 0b9695125c0a9..8aaae4644415b 100644 --- a/doc/source/whatsnew/v0.19.0.txt +++ b/doc/source/whatsnew/v0.19.0.txt @@ -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`) \ No newline at end of file diff --git a/pandas/core/strings.py b/pandas/core/strings.py index 6ec28f9735850..a664673b2abdd 100644 --- a/pandas/core/strings.py +++ b/pandas/core/strings.py @@ -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}' + raise TypeError(msg.format(type(width).__name__)) + if side == 'left': f = lambda x: x.rjust(width, fillchar) elif side == 'right': diff --git a/pandas/tests/test_strings.py b/pandas/tests/test_strings.py index 4d23bed620265..7bc34304d4b70 100644 --- a/pandas/tests/test_strings.py +++ b/pandas/tests/test_strings.py @@ -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): + + values = Series(['1', '22', 'a', 'bb']) + + result = values.str.pad(5, side='left', fillchar='0') + expected = Series(['00001', '00022', '0000a', '000bb']) + tm.assert_almost_equal(result, expected) + + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type, not*"): + result = values.str.pad('f', fillchar='0') + def test_translate(self): def _check(result, expected): From 06795dbd11c51e39964d86f6972227f2e66ca0bb Mon Sep 17 00:00:00 2001 From: wcwagner Date: Sun, 17 Jul 2016 20:24:45 -0400 Subject: [PATCH 2/4] BUG: "Added tests for width parameter on center, ljust, rjust, zfill." --- doc/source/whatsnew/v0.19.0.txt | 2 +- pandas/tests/test_strings.py | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.19.0.txt b/doc/source/whatsnew/v0.19.0.txt index 8aaae4644415b..5c4c1126cb078 100644 --- a/doc/source/whatsnew/v0.19.0.txt +++ b/doc/source/whatsnew/v0.19.0.txt @@ -614,4 +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`) \ No newline at end of file +- Bug in ``pandas.Series.str.zfill``, when ``width`` was not of integer type no ``TypeError`` was raised, which resulted in a series with all NaN values (:issue:`13598`) \ No newline at end of file diff --git a/pandas/tests/test_strings.py b/pandas/tests/test_strings.py index 7bc34304d4b70..fbe9d13914cb0 100644 --- a/pandas/tests/test_strings.py +++ b/pandas/tests/test_strings.py @@ -1757,6 +1757,33 @@ def test_center_ljust_rjust_fillchar(self): "fillchar must be a character, not int"): result = values.str.rjust(5, fillchar=1) + def test_center_ljust_rjust_width(self): + values = Series(['a', 'bb', 'ccc', NA, 'eeeee']) + + result = values.str.center(4, fillchar='X') + expected = Series(['XaXX', 'XbbX', 'cccX', NA, 'eeeee']) + tm.assert_almost_equal(result, expected) + + result = values.str.ljust(4, fillchar='X') + expected = Series(['aXXX', 'bbXX', 'cccX', NA, 'eeeee']) + tm.assert_almost_equal(result, expected) + + result = values.str.rjust(4, fillchar='X') + expected = Series(['XXXa', 'XXbb', 'Xccc', NA, 'eeeee']) + tm.assert_almost_equal(result, expected) + + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type, not*"): + result = values.str.center('f', fillchar='X') + + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type, not*"): + result = values.str.ljust('f', fillchar='X') + + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type, not*"): + result = values.str.rjust('f', fillchar='X') + def test_zfill(self): values = Series(['1', '22', 'aaa', '333', '45678']) @@ -1779,6 +1806,17 @@ def test_zfill(self): expected = Series(['00001', np.nan, '00aaa', np.nan, '45678']) tm.assert_series_equal(result, expected) + def test_zfill_width(self): + values = Series(['1', '22', 'ccc', NA, 'eeeee']) + + result = values.str.zfill(5) + expected = Series(['00001', '00022', '00ccc', NA, 'eeeee']) + tm.assert_almost_equal(result, expected) + + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type, not*"): + result = values.str.zfill('f') + def test_split(self): values = Series(['a_b_c', 'c_d_e', NA, 'f_g_h']) From 40a3188e3d7bf142f14b388c864c63556d3e6929 Mon Sep 17 00:00:00 2001 From: wcwagner Date: Sun, 17 Jul 2016 22:30:23 -0400 Subject: [PATCH 3/4] BUG: "Switched to single test method asserting functions that use pad raise correctly." --- pandas/tests/test_strings.py | 52 +++++------------------------------- 1 file changed, 6 insertions(+), 46 deletions(-) diff --git a/pandas/tests/test_strings.py b/pandas/tests/test_strings.py index fbe9d13914cb0..f0d191195948c 100644 --- a/pandas/tests/test_strings.py +++ b/pandas/tests/test_strings.py @@ -1604,16 +1604,14 @@ def test_pad_fillchar(self): result = values.str.pad(5, fillchar=5) def test_pad_width(self): - values = Series(['1', '22', 'a', 'bb']) + string_methods = Series.str(values) - result = values.str.pad(5, side='left', fillchar='0') - expected = Series(['00001', '00022', '0000a', '000bb']) - tm.assert_almost_equal(result, expected) - - with tm.assertRaisesRegexp(TypeError, - "width must be of integer type, not*"): - result = values.str.pad('f', fillchar='0') + for f_name, f in Series.str.__dict__.items(): + if f_name in ['center', 'ljust', 'rjust', 'zfill', 'pad']: + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type,*"): + f(string_methods, 'f') def test_translate(self): @@ -1757,33 +1755,6 @@ def test_center_ljust_rjust_fillchar(self): "fillchar must be a character, not int"): result = values.str.rjust(5, fillchar=1) - def test_center_ljust_rjust_width(self): - values = Series(['a', 'bb', 'ccc', NA, 'eeeee']) - - result = values.str.center(4, fillchar='X') - expected = Series(['XaXX', 'XbbX', 'cccX', NA, 'eeeee']) - tm.assert_almost_equal(result, expected) - - result = values.str.ljust(4, fillchar='X') - expected = Series(['aXXX', 'bbXX', 'cccX', NA, 'eeeee']) - tm.assert_almost_equal(result, expected) - - result = values.str.rjust(4, fillchar='X') - expected = Series(['XXXa', 'XXbb', 'Xccc', NA, 'eeeee']) - tm.assert_almost_equal(result, expected) - - with tm.assertRaisesRegexp(TypeError, - "width must be of integer type, not*"): - result = values.str.center('f', fillchar='X') - - with tm.assertRaisesRegexp(TypeError, - "width must be of integer type, not*"): - result = values.str.ljust('f', fillchar='X') - - with tm.assertRaisesRegexp(TypeError, - "width must be of integer type, not*"): - result = values.str.rjust('f', fillchar='X') - def test_zfill(self): values = Series(['1', '22', 'aaa', '333', '45678']) @@ -1806,17 +1777,6 @@ def test_zfill(self): expected = Series(['00001', np.nan, '00aaa', np.nan, '45678']) tm.assert_series_equal(result, expected) - def test_zfill_width(self): - values = Series(['1', '22', 'ccc', NA, 'eeeee']) - - result = values.str.zfill(5) - expected = Series(['00001', '00022', '00ccc', NA, 'eeeee']) - tm.assert_almost_equal(result, expected) - - with tm.assertRaisesRegexp(TypeError, - "width must be of integer type, not*"): - result = values.str.zfill('f') - def test_split(self): values = Series(['a_b_c', 'c_d_e', NA, 'f_g_h']) From 9669f3f139445a98d38d7feee1d12ab04fe0362d Mon Sep 17 00:00:00 2001 From: wcwagner Date: Mon, 18 Jul 2016 19:06:58 -0400 Subject: [PATCH 4/4] BUG: "Replaced isinstance with is_integer, and changed test_pad_width to use getattr" --- pandas/core/strings.py | 5 +++-- pandas/tests/test_strings.py | 11 +++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pandas/core/strings.py b/pandas/core/strings.py index a664673b2abdd..3150fc5d0143a 100644 --- a/pandas/core/strings.py +++ b/pandas/core/strings.py @@ -8,7 +8,8 @@ is_object_dtype, is_string_like, is_list_like, - is_scalar) + is_scalar, + is_integer) from pandas.core.common import _values_from_object from pandas.core.algorithms import take_1d @@ -914,7 +915,7 @@ 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): + if not is_integer(width): msg = 'width must be of integer type, not {0}' raise TypeError(msg.format(type(width).__name__)) diff --git a/pandas/tests/test_strings.py b/pandas/tests/test_strings.py index f0d191195948c..2a4fc8b2db5c9 100644 --- a/pandas/tests/test_strings.py +++ b/pandas/tests/test_strings.py @@ -1605,13 +1605,12 @@ def test_pad_fillchar(self): def test_pad_width(self): values = Series(['1', '22', 'a', 'bb']) - string_methods = Series.str(values) + s = Series(values) - for f_name, f in Series.str.__dict__.items(): - if f_name in ['center', 'ljust', 'rjust', 'zfill', 'pad']: - with tm.assertRaisesRegexp(TypeError, - "width must be of integer type,*"): - f(string_methods, 'f') + for f in ['center', 'ljust', 'rjust', 'zfill', 'pad']: + with tm.assertRaisesRegexp(TypeError, + "width must be of integer type, not*"): + getattr(s.str, f)('f') def test_translate(self):