[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: Since the op codes are singletons, you can use identity tests instead of equality checks in sre_parse.py: Please ignore my reply in previous message. Op codes are always tested for identity in sre_compile.py, so I have applied your suggestion in

[issue22434] Use named constants internally in the re module

Roundup Robot added the comment: New changeset fc7dbba57869 by Serhiy Storchaka in branch 'default': Issue #22434: Constants in sre_constants are now named constants (enum-like). https://hg.python.org/cpython/rev/fc7dbba57869 -- nosy: +python-dev ___

[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: Thank you Raymond for the review. I especially like the removal of superfluous OPCODES dictionary lookups The disadvantage is that now backporting patches to old branches is harder. Since the op codes are singletons, you can use identity tests instead of

[issue22434] Use named constants internally in the re module

Changes by Serhiy Storchaka storch...@gmail.com: -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22434 ___

[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: Could you please make a review of any patch Antoine? This would help me to debug re engine. It doesn't matter which patch apply, with good chance all this will be changed before 3.5 release and may be not once. --

[issue22434] Use named constants internally in the re module

Raymond Hettinger added the comment: I reviewed re_named_consts.patch and it looks great (I especially like the removal of superfluous OPCODES dictionary lookups and improved repr for the integer codes). Since the op codes are singletons, you can use identity tests instead of equality checks

[issue22434] Use named constants internally in the re module

Changes by Raymond Hettinger raymond.hettin...@gmail.com: -- nosy: +effbot ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22434 ___ ___

[issue22434] Use named constants internally in the re module

Antoine Pitrou added the comment: I think I agree with Serhiy. Augmenting the import dependencies from the re module can have side effects. While the alternative (a custom Int subclass) isn't extremely pretty, it does the job. NamedIntConstant should be private, though. --

[issue22434] Use named constants internally in the re module

Guido van Rossum added the comment: OK, then if you want to review Serhiy's original patch that would be great. On Sun, Sep 21, 2014 at 5:12 AM, Antoine Pitrou rep...@bugs.python.org wrote: Antoine Pitrou added the comment: I think I agree with Serhiy. Augmenting the import dependencies

[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: NamedIntConstant should be private, though. Agree. I'll add underscore to NamedIntConstant and makecode before committing (as in enum patch). -- ___ Python tracker rep...@bugs.python.org

[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: Here is a patch using enums. I still think enums are superfluous here. Advanced the Enum class features (pickling, access by name, type checking) are not needed - these constants don't leaked in outer word. I don't see where there would be a circular

[issue22434] Use named constants internally in the re module

Changes by Terry J. Reedy tjre...@udel.edu: -- nosy: +terry.reedy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22434 ___ ___ Python-bugs-list

[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: If there are no objections I'll commit the patch soon. -- assignee: - serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22434 ___

[issue22434] Use named constants internally in the re module

Guido van Rossum added the comment: Hm. Could you not use the new Enum class? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22434 ___ ___

[issue22434] Use named constants internally in the re module

Serhiy Storchaka added the comment: Answering Guido's question about the Enum class. No, it is not appropriate here. It has too cumbersome repr (OPCODES.IN_IGNORE: 16 instead of IN_IGNORE). Enum function syntax can't by used because it enumerates values from 1. We need three Enum subclasses

[issue22434] Use named constants internally in the re module

Guido van Rossum added the comment: I think you are too casual in rejecting a standard approach over a custom clever hack. Making the values enums gives them a standard interface that goes beyond what you implemented, and just the fact that we can say these are IntEnum instances specifies a lot

[issue22434] Use named constants internally in the re module

New submission from Serhiy Storchaka: Regular expression parser parses a pattern to a tree, marking nodes by string identifiers. Regular expression compiler converts this three into plain list of integers. Node's identifiers are transformed to sequential integers. Resulting list is not human

[issue22434] Use named constants internally in the re module

Guido van Rossum added the comment: Love it! -- nosy: +gvanrossum ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22434 ___ ___ Python-bugs-list