Code Sample (pandas 0.23.0)
In [1]:
import pandas as pd
s = pd.Series(['abc','123'])
s.str.replace('.','',regex = True)
Out [1]:
0 abc
1 123
dtype: object
Problem description
Hi everyone, I was showing str.replace to a colleague and how it uses regex by default and when I entered pd.Series.str.replace('.','') I expected every character to be removed but instead nothing happened. Is that a bug or is there something I don't understand?
Expected Output: empty strings ""
Out [1]:
0
1
dtype: object
Output of pandas 0.23.0
Comment From: WillAyd
Hmm interesting. As a workaround you could do the following:
s.str.replace('.*', '', regex = True)
Though you are correct in that the wildcard expression shouldn't be necessary, given the following works:
re.sub('.', '', 'abc')
Investigation and PRs are always welcome
Comment From: ThibTrip
Hmm interesting. As a workaround you could do the following:
python s.str.replace('.*', '', regex = True)Though you are correct in that the wildcard expression shouldn't be necessary, given the following works:
python re.sub('.', '', 'abc')Investigation and PRs are always welcome
Hi WillAyd, thanks for your input. I did not realise I was not using the last version but with 0.23.4 I have the same issue as well. My goal is to help pandas so while having a workaround is nice if it's a bug then I'll be happy to see it fixed :) (of course it would be better with a pull request but that seems quite hard).
Comment From: WillAyd
If you want to poke around in the implementation I think the right starting point is here:
https://github.com/pandas-dev/pandas/blob/98f46f7f08e9f15374fd840507438cb237313a9e/pandas/core/strings.py#L428
We also have a contributing guide to help walk through your first PR:
https://pandas.pydata.org/pandas-docs/stable/contributing.html
Comment From: alarangeiras
Hi,
I looked into this question and noticed two things:
1) @ThibTrip, the way you use this method (s.str.replace('.','',regex = True)) is a little mistaken. You need to reassign the result of this method to the varible "s" (or another varible of your choice). Just like below:
import pandas as pd
s = pd.Series(['abc','123'])
s = s.str.replace('.','teste',regex = True)
print(s)
2) Besides that, I found a bug on method str_replace just like mentioned by @WillAyd. This method considered a pattern size greater than 1. The size of the period (=1) was ignored.
if is_compiled_re or len(pat) > 1 or flags or callable(repl):
n = n if n >= 0 else 0
compiled = re.compile(pat, flags=flags)
f = lambda x: compiled.sub(repl=repl, string=x, count=n)
else:
f = lambda x: x.replace(pat, repl, n)
I only changed from
len(pat) > 1
to
len(pat) > 0
I fixed and tested this change, can i make a PR?
Comment From: WillAyd
@alarangeiras thanks for the investigation! Sure if you want to submit a PR would be great - you can use the contributing guide above to help you through that. Just be sure to add a test case with the change
Comment From: alarangeiras
@WillAyd, Sure, i'll submit a test case too.
Comment From: Liam3851
I added the regex=False default kwarg to .str.replace (see #16808, #19584). Prior to that it was always regex for len > 1 and literal for len == 1 (though undocumented as such). I noted the potential confusion at https://github.com/pandas-dev/pandas/issues/16808#issuecomment-362670420 but kept it as-is for back compat, absent any response from the reviewers. So this is (somewhat) expected behavior, though I agree inconsistent with the current documentation.
As I noted there I think the only case where this makes a difference is if someone does a .str.replace('.', otherstring) to replace every character with otherstring. I think that's probably very rare in anything but the toy use case listed here?
I think this is definitely an acceptable change to avoid the confusion outlined here, but I would suggest it should be made with a deprecation cycle (warn on .replace of single character with regex unspecified, at least if that is a special character). Any production users currently calling .str.replace('.', '') are expecting to strip periods, not clear the string, for example, and this has been the behavior going back to the introduction of the function.
Comment From: ThibTrip
I added the
regex=Falsedefault kwarg to .str.replace (see #16808, #19584). Prior to that it was always regex for len > 1 and literal for len == 1 (though undocumented as such). I noted the potential confusion at #16808 (comment) but kept it as-is for back compat, absent any response from the reviewers. So this is (somewhat) expected behavior, though I agree inconsistent with the current documentation.As I noted there I think the only case where this makes a difference is if someone does a .str.replace('.', otherstring) to replace every character with otherstring. I think that's probably very rare in anything but the toy use case listed here?
I think this is definitely an acceptable change to avoid the confusion outlined here, but I would suggest it should be made with a deprecation cycle (warn on .replace of single character with regex unspecified, at least if that is a special character). Any production users currently calling .str.replace('.', '') are expecting to strip periods, not clear the string, for example, and this has been the behavior going back to the introduction of the function.
Hi :)! Sorry I was very busy but I did see the code and understand that a regex pattern of length one is a weird use case. I like the idea of warning when the pattern is only one special character (".","^", "$"...) and regex argument is not provided.
Comment From: TomAugspurger
It looks like the WIP PR https://github.com/pandas-dev/pandas/pull/24809 has been closed.
If anyone is interested in picking this up that will be a good starting point. The main remaining issue is discussed in https://github.com/pandas-dev/pandas/pull/24809#discussion_r248545924. We don't want to change behavior immediately. We need to deprecate the old behavior first.
Comment From: simonjayhawkins
If anyone is interested in picking this up that will be a good starting point. The main remaining issue is discussed in #24809 (comment). We don't want to change behavior immediately. We need to deprecate the old behavior first.
done in #36695