BUG: Fix for .str.replace with invalid input #13460
Conversation
Current coverage is 84.28%@@ master #13460 diff @@
==========================================
Files 138 138
Lines 50937 50939 +2
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 42934 42936 +2
Misses 8003 8003
Partials 0 0
|
| @@ -81,3 +81,4 @@ Performance Improvements | |||
|
|
|||
| Bug Fixes | |||
| ~~~~~~~~~ | |||
| Bug in ``.str.replace`` does not raise when replacement is None (:issue:`13438`) | |||
There was a problem hiding this comment.
Pls describe the kind of error, like "does not raise ValueError".
sinhrks
added
Error Reporting
priyankjain
force-pushed
the
13438
branch
3 times, most recently
from
0e398b6 to
f4f04e3
Compare
| return _na_map(f, arr) | ||
| return _na_map(f, arr) | ||
| else: | ||
| raise TypeError("Can't convert {0} object to str implicitly" |
There was a problem hiding this comment.
Can you put this in the beginning? I (so switch the if/else part, also in that case the else is not needed)
IMO that is an easier to follow logic.
There was a problem hiding this comment.
Further, I don't find the error message clear. I know this is the one from the stdlib, but a numeric value can be converted to string, so that sounds a bit strange. Maybe just something like "expected a string" or "the replacement argument should be a string"
There was a problem hiding this comment.
Will do said changes, do you know why travis test has not started, it's yellow for around half an hour, no progress.
There was a problem hiding this comment.
That is because there is a queue, and each time you push a new version (or somebody else pushes on his/her PR), one travis build is added to this queue: https://travis-ci.org/pydata/pandas/pull_requests
(but will stop some of your builds)
priyankjain
force-pushed
the
13438
branch
2 times, most recently
from
781b959 to
4079333
Compare
| @@ -309,6 +309,8 @@ def str_replace(arr, pat, repl, n=-1, case=True, flags=0): | |||
| ------- | |||
| replaced : Series/Index of objects | |||
| """ | |||
| if not is_string_like(repl): # Check whether repl is valid (GH 13438) | |||
| raise TypeError("Replacement should be a string") | |||
There was a problem hiding this comment.
how about "repl must be a string", to be consistent with docstring.
.str.replace now raises TypeError when replacement string is not string like
|
thanks @priyankjain |
git diff upstream/master | flake8 --diff.str.replace now raises TypeError when replacement is invalid