re.escape should not escape underscore

@birkenfeld

BPO	2650
Nosy	@birkenfeld, @amauryfa, @abalkin, @pitrou, @benjaminp, @ezio-melotti
Files	re.patch: patch to python svn trunk (2.6a2+) re.patch re.patch: patch using enumerate and frozenset re_patch.diff: Compilation of previous patches, testing for more characters included on the ASCII table test_re.diff test_re.diff issue2650.diff: Patch to add '_' to the non-escaped chars.

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ezio-melotti'
closed_at = <Date 2011-04-10.10:00:23.726>
created_at = <Date 2008-04-17.14:14:08.568>
labels = ['expert-regex', 'easy', 'type-bug']
title = 're.escape should not escape underscore'
updated_at = <Date 2011-04-10.10:00:23.726>
user = 'https://bugs.python.org/rsc'

bugs.python.org fields:

activity = <Date 2011-04-10.10:00:23.726>
actor = 'ezio.melotti'
assignee = 'ezio.melotti'
closed = True
closed_date = <Date 2011-04-10.10:00:23.726>
closer = 'ezio.melotti'
components = ['Regular Expressions']
creation = <Date 2008-04-17.14:14:08.568>
creator = 'rsc'
dependencies = []
files = ['10080', '10084', '10130', '10215', '20860', '21096', '21390']
hgrepos = []
issue_num = 2650
keywords = ['patch', 'easy']
message_count = 43.0
messages = ['65585', '65590', '65599', '65600', '65708', '65721', '65923', '66386', '66416', '66418', '66419', '66420', '66421', '66422', '66423', '68208', '68785', '68786', '68895', '68910', '92548', '122408', '125746', '125770', '125778', '126138', '126141', '126168', '126176', '126177', '126181', '126184', '126185', '130719', '130783', '130821', '130930', '132082', '132084', '132085', '132087', '132838', '133461']
nosy_count = 17.0
nosy_names = ['georg.brandl', 'amaury.forgeotdarc', 'belopolsky', 'foom', 'pitrou', 'rsc', 'timehorse', 'benjamin.peterson', 'zanella', 'donlorenzo', 'ezio.melotti', 'bjourne', 'mortenlj', 'mrabarnett', 'SilentGhost', 'swamiyeswanth', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue2650'
versions = ['Python 3.2']

import re
print re.escape("_")

Prints \_ but should be _.

This behavior differs from Perl and other systems: _ is an identifier
character and as such does not need to be escaped.

It seems that escape is pretty dumb. The documentations says that
re.escape escapes all non-alphanumeric characters, and it does that
faithfully. It would seem more useful to have a list of meta-characters
and just escape those. This is more true in Py3k when str can have
thousands of possible characters that could be considered alphanumeric.

It seems that escape is pretty dumb. The documentations says that
re.escape escapes all non-alphanumeric characters, and it does that
faithfully. It would seem more useful to have a list of meta-characters
and just escape those. This is more true in Py3k when str can have
thousands of possible characters that could be considered alphanumeric.

The usual convention is to escape everything that is
ASCII and not A-Za-z0-9_, in case other punctuation
becomes special in the future. But I agree -- escaping
just the actual special characters makes the most sense.

Russ

Would you like to work on a patch?

Thanks.

The loop in escape should really use enumerate instead of "for i in
range(len(pattern))".

Instead of using a loop, can't the test just use
"self.assertEqual(re.esacpe(same), same)?" Also, please add tests for
what re.escape should escape.

The loop in escape should really use enumerate
instead of "for i in range(len(pattern))".

It needs i to edit s[i].

Instead of using a loop, can't the test just
use "self.assertEqual(re.esacpe(same), same)?"

Done.

Also, please add tests for what re.escape should escape.

That's handled in the existing test over all bytes 0-255.

> The loop in escape should really use enumerate
> instead of "for i in range(len(pattern))".

It needs i to edit s[i].

enumerate(iterable) returns a tuple for each element in iterable
containing the index and the element itself.

I attached a patch using enumerate. The patch also uses a frozenset
rather than a dict for the special characters.

AFAIK the lookup on dictionaries is faster than on lists.

Patch added, mainly a compilation of the previous patches with an
expanded test.

Lorenz's patch uses a set, not a list for special characters. Set
lookup is as fast as dict lookup, but a set takes less memory because it
does not have to store dummy values. More importantly, use of frozenset
instead of dict makes the code clearer. On the other hand, I would
simply use a string. For a dozen entries, hash lookup does not buy you
much.

Another nit: why use "\\%c" % (c) instead of obvious "\\" + c?

Finally, you can eliminate use of index and a temporary list altogether
by using a generator expression:

''.join(("\\" + c if c in _special else '\\000' if c == "\000" else c),
for c in pattern)

Lorenz's patch uses a set, not a list for special characters. Set
lookup is as fast as dict lookup, but a set takes less memory because it
does not have to store dummy values. More importantly, use of frozenset
instead of dict makes the code clearer. On the other hand, I would
simply use a string. For a dozen entries, hash lookup does not buy you
much.

Another nit: why use "\\%c" % (c) instead of obvious "\\" + c?

Finally, you can eliminate use of index and a temporary list altogether
by using a generator expression:

''.join(("\\" + c if c in _special else '\\000' if c == "\000" else c),
for c in pattern)

The title of this issue (bpo-2650) is "re.escape should not escape underscore",
not "re.escape is too slow and too easy to read".

If you have an actual, measured performance problem with re.escape,
please open a new issue with numbers to back it up.
That's not what this one is about.

Thanks.
Russ

On Thu, May 8, 2008 at 10:36 AM, Russ Cox <report@bugs.python.org> wrote:
..

The title of this issue (bpo-2650) is "re.escape should not escape underscore",
not "re.escape is too slow and too easy to read".

Neither does the title say "re.escape should only escape
.^$*+?{}[]\|()". I reviewed the patch rather than its conformance
with the title. (BTW, the patch does not update documentation in
Doc/library/re.rst.)

If you have an actual, measured performance problem with re.escape,
please open a new issue with numbers to back it up.
That's not what this one is about.

You don't need to get so defensive. I did not raise a performance
problem, I was simply responding to Rafael's "AFAIK the lookup on
dictionaries is faster than on lists" comment. I did not say that you
*should* rewrite your patch the way I suggested, only that you *can*
use new language features to simplify the code.

In any case, I am -0 on the patch. The current documentation says:

"""
escape(string)

Return *string* with all non-alphanumerics backslashed; this is useful if you
want to match an arbitrary literal string that may have regular expression
metacharacters in it.

"""

and the current implementation serves the intended use case well. I
did not see a compelling use case presented for the change. On the
downside, since there is no mechanism to assure that _special indeed
contains all re metacharacters, it may present a maintenance problem
if additional metacharacters are added in the future.

You don't need to get so defensive. I did not raise a performance
problem, I was simply responding to Rafael's "AFAIK the lookup on
dictionaries is faster than on lists" comment. I did not say that you
*should* rewrite your patch the way I suggested, only that you *can*
use new language features to simplify the code.

I was responding to the entire thread more than your mail.
I'm frustrated because the only substantial discussion has
focused on details of how to implement set lookup the fastest
in a function that likely doesn't matter for speed.

In any case, I am -0 on the patch. The current documentation says:

Now these are the kinds of comments I was hoping for.
Thank you.

Return *string* with all non-alphanumerics backslashed; this is useful if you
want to match an arbitrary literal string that may have regular expression
metacharacters in it.

Sure; the documentation is wrong too.

I did not see a compelling use case presented for the change.

The usual convention in regular expressions is that escaping
a word character means you intend a special meaning, and
underscore is a word character. Even though the current re
module does accept \_ as synonymous with _ (just as it accepts
\q as synonymous with q), it is no more correct to escape _ than
to escape q.

I think it is fine to escape all non-word characters, but someone
else suggested that it would be easier when moving to larger
character sets to escape just the special ones. I'm happy with
either version.

My argument is only that Python should behave the same in
this respect as other systems that use substantially the same
regular expressions.

since there is no mechanism to assure that _special indeed
contains all re metacharacters, it may present a maintenance problem
if additional metacharacters are added in the future.

The test suite will catch these easily, since it checks that
re.escape(c) matches c for all characters c. But again, I'm happy
with escaping all ASCII non-word characters.

Russ

Right you are, it seems that python's regexp implementation is terribly slow when doing replacements with a substitution in them. (fixing the broken test, as you pointed out changed the timing to 97.6 usec vs the in-error-reported 18.3usec.)

Oh well. I still think it's crazy not to use re for this in its own module. Someone just needs to fix re to not be horrifically slow, too. :)

@james

@james test results for py3k

python -m timeit -s "$(printf "import re\ndef escape(s):\n return re.sub('([][.^$+?{}\\|()])', '\\\1', s)")" 'escape("!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&*()")'

100000 loops, best of 3: 17.1 usec per loop

python -m timeit -s "import re" 're.escape("!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()!@#$%^&()")'

10000 loops, best of 3: 102 usec per loop

Here is the latest patch for test_re incorporating review suggestions by Ezio and some improvements along the way.

I took a look to what other languages do, and it turned out that:

perl escapes [^A-Za-z_0-9] 0;
.net escapes the metachars and whitespace 1;
java escapes the metachars or escape sequences 2;
ruby escapes the metachars 3;

It might be OK to exclude _ from the escaped chars, but I would keep escaping all the other non-alnum chars too (i.e. match perl behavior).

(FWIW, I don't think re.escape() is used in performance-critical situation, so readability should probably be preferred over speed.)

I think these are two different questions:

What to escape
What to do about poor performance of the re.escape when re.sub is used

In my opinion, there isn't any justifiable reason to escape non-meta characters: it doesn't affect matching; escaped strings are typically just re-used in regex.

I would favour simpler and cleaner code with re.sub. I don't think that re.quote could be a performance bottleneck in any application. I did some profiling with python3.2 and it seems that the reason for this poor performance is many abstraction layers when using re.sub. However, we need to bear in mind that we're only talking about 40 usec difference for a 100-char string (string.printable): I'd think that strings being escaped are typically shorter.

As a compromise, I tested this code:

_mp = {ord(i): '\\' + i for i in '][.^$*+?{}\\|()'}

def escape(pattern):
    if isinstance(pattern, str):
        return pattern.translate(_mp)
    return sub(br'([][.^$*+?{}\\|()])', br'\\\1', pattern)

which is fast (faster than existing code) for str and slow for bytes patterns.
I don't particularly like it, because of the difference between str and bytes handling, but I do think that it will be much easier to "fix" once/when/if re module is improved.

re.escape and its tests can be refactored in 2.7/3.1, the '_' can be added to the list of chars that are not escaped in 3.3.

I'll put together a patch and fix this unless someone thinks that the '_' should be escaped in 3.3 too.

New changeset 1402c719b7cf by Ezio Melotti in branch '3.1':
bpo-2650: Refactor the tests for re.escape.
http://hg.python.org/cpython/rev/1402c719b7cf

New changeset 9147f7ed75b3 by Ezio Melotti in branch '3.1':
bpo-2650: Add tests with non-ascii chars for re.escape.
http://hg.python.org/cpython/rev/9147f7ed75b3

New changeset ed02db9921ac by Ezio Melotti in branch '3.1':
bpo-2650: Refactor re.escape to use enumerate().
http://hg.python.org/cpython/rev/ed02db9921ac

New changeset 42ab3ebb8c2c by Ezio Melotti in branch '3.2':
bpo-2650: Merge with 3.1.
http://hg.python.org/cpython/rev/42ab3ebb8c2c

New changeset 9da300ad8255 by Ezio Melotti in branch 'default':
bpo-2650: Merge with 3.2.
http://hg.python.org/cpython/rev/9da300ad8255

New changeset d52b1faa7b11 by Ezio Melotti in branch '2.7':
bpo-2650: Refactor re.escape and its tests.
http://hg.python.org/cpython/rev/d52b1faa7b11

I did a few more tests and using a re.sub seems indeed slower (the implementation is just 4 lines though, and it's more readable):

wolf@hp:/dev/py/3.1$ ./python -m timeit -s 'import re,string; escape_pattern = re.compile("([^\\x00a-zA-Z0-9])")' 'escape_pattern.sub(r"\\\1", string.printable).replace("\x00", "\\000")'
1000 loops, best of 3: 219 usec per loop
wolf@hp:/dev/py/3.1$ ./python -m timeit -s 'import re,string' 're.escape(string.printable)'
10000 loops, best of 3: 59.3 usec per loop
wolf@hp:~/dev/py/3.1$ ./python -c 'import re,string; escape_pattern = re.compile("([^\\x00a-zA-Z0-9])"); print(escape_pattern.sub(r"\\\1", string.printable).replace("\x00", "\\000") == re.escape(string.printable))'
True

wolf@hp:/dev/py/3.1$ ./python -m timeit -s 'import re,string; escape_pattern = re.compile(b"([^\\x00a-zA-Z0-9])"); s = string.printable.encode("ascii")' 'escape_pattern.sub(br"\\\1", s).replace(b"\x00", b"\\000")'
1000 loops, best of 3: 231 usec per loop
wolf@hp:/dev/py/3.1$ ./python -m timeit -s 'import re,string; s = string.printable.encode("ascii")' 're.escape(s)'
10000 loops, best of 3: 73.2 usec per loop
wolf@hp:~/dev/py/3.1$ ./python -c 'import re,string; escape_pattern = re.compile(b"([^\\x00a-zA-Z0-9])"); s = string.printable.encode("ascii"); print(escape_pattern.sub(br"\\\1", s).replace(b"\x00", b"\\000") == re.escape(s))'
True

The .replace() doesn't seem to affect the affect the speed in any significant way.
I also did a few more tests:

using enumerate();
like 1) but also moving \x00 in the set of alnum chars, removing the "if c == '\000'" from the loop and using .replace("\x00", "\\000") on the joined string;
like 2) but also moving the loop in a genexp inside the join();
is the fastest (10-15% faster than the original), 2) is pretty much the same speed of 1), and 3) is slower, so I just changed re.escape to use enumerate() and refactored its tests in 2.7/3.1/3.2/3.3.

The attached patch (bpo-2650.diff) adds '_' to the list of chars that are not escaped.

Georg, do you think a versionchanged note should be added for this? The change is minor and the patch updates the documentation to reflect the change.

New changeset dda33191f7f5 by Ezio Melotti in branch 'default':
bpo-2650: re.escape() no longer escapes the "_".
http://hg.python.org/cpython/rev/dda33191f7f5

Navigation Menu

Repository navigation

re.escape should not escape underscore #46902

19 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

re.escape should not escape underscore #46902

Description

Activity

rsc commented on Apr 17, 2008

benjaminp commented on Apr 17, 2008

rsc commented on Apr 18, 2008

benjaminp commented on Apr 18, 2008

benjaminp commented on Apr 24, 2008

rsc commented on Apr 24, 2008

donlorenzo commented on Apr 28, 2008

zanella commented on May 8, 2008

abalkin commented on May 8, 2008

rsc commented on May 8, 2008

abalkin commented on May 8, 2008

rsc commented on May 8, 2008

19 remaining items

foom commented on Jan 13, 2011

swamiyeswanth commented on Jan 13, 2011

SilentGhost commented on Mar 13, 2011

ezio-melotti commented on Mar 14, 2011

SilentGhost commented on Mar 14, 2011

ezio-melotti commented on Mar 14, 2011

python-dev commented on Mar 25, 2011

python-dev commented on Mar 25, 2011

ezio-melotti commented on Mar 25, 2011

ezio-melotti commented on Mar 25, 2011

ezio-melotti commented on Apr 3, 2011

python-dev commented on Apr 10, 2011

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions