Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

re: convert re flags to (much friendlier) IntFlag constants #72269

Closed
ethanfurman opened this issue Sep 11, 2016 · 16 comments
Closed

re: convert re flags to (much friendlier) IntFlag constants #72269

ethanfurman opened this issue Sep 11, 2016 · 16 comments
Assignees
@ethanfurman
Labels
type-feature A feature request or enhancement

Comments

@ethanfurman
Copy link
Member

BPO 28082
Nosy @gvanrossum, @rhettinger, @vstinner, @ethanfurman, @serhiy-storchaka
Files
  • issue-re.stoneleaf.02.patch
    		•

    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/ethanfurman'
closed_at = <Date 2016-11-21.16:31:14.567>
created_at = <Date 2016-09-11.20:29:30.011>
labels = ['type-feature']
title = 're: convert re flags to (much friendlier) IntFlag constants'
updated_at = <Date 2016-11-21.16:40:28.001>
user = 'https://github.com/ethanfurman'

    bugs.python.org fields:

    activity = <Date 2016-11-21.16:40:28.001>
actor = 'python-dev'
assignee = 'ethan.furman'
closed = True
closed_date = <Date 2016-11-21.16:31:14.567>
closer = 'python-dev'
components = []
creation = <Date 2016-09-11.20:29:30.011>
creator = 'ethan.furman'
dependencies = []
files = ['44566']
hgrepos = []
issue_num = 28082
keywords = ['patch']
message_count = 16.0
messages = ['275848', '275849', '275860', '275863', '275864', '275865', '275869', '275880', '275881', '275883', '280257', '280263', '280754', '280836', '281377', '281380']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'rhettinger', 'vstinner', 'ethan.furman', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28082'
versions = ['Python 3.6']
    @ethanfurman
    Copy link
    Member Author

    Split from bpo-23591.

    @ethanfurman ethanfurman added the type-feature A feature request or enhancement label Sep 11, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 11, 2016

    New changeset 223731925d06 by Ethan Furman in branch 'default':
    bpo-28082: use IntFlag for re constants
    https://hg.python.org/cpython/rev/223731925d06

    @rhettinger
    Copy link
    Contributor

    Guido, is this something you wanted to happen? I thought you had objected to propagating the four flavors of enum throughout the standard library, particularly for long standing, stable APIs.

    AFAICT, no one has ever requested this for the re module, nor is there any demonstrated need. As a heavy user of regexes, I've have never looked at the flag values (and if I had, it wouldn't have been helpful to hide that these are integer values rather than giving them both a new type and an unattractive appearance: <Flag.ASCII|IGNORECASE: 258>. Also, prior to this change, the re module and its sre components had no external dependencies and did not require any other modules to be loaded in memory to run.

    If changes like this do go in, it needs better names (i.e. Flag -> RegexFlag) so that someone using typing doesn't end-up many distinct kinds of integer flags all being called Flag.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 11, 2016

    New changeset 7369ec91d0f7 by Ethan Furman in branch 'default':
    bpo-28082: better name for Flag
    https://hg.python.org/cpython/rev/7369ec91d0f7

    @ethanfurman
    Copy link
    Member Author

    The patch was initially from Serhiy as part of bpo-23591. So it's safe to say at least one person requested it, and a core dev at that.

    I will happily make it two people: as an occasional user of re having the constants be named makes it much easier for me to use; I daresay that is true for other occasional users. IIRC giving names to numbers was one of the motivating factors in having Enum in the first place.

    I do agree that RegexFlag is a better name -- I wasn't real happy with Flag but didn't want to miss the cutoff.

    @serhiy-storchaka
    Copy link
    Member

    re flags was the primary motive of introducing general IntFlags. This would help to handle frequent user error. Original issue is bpo-11957.

    @ethanfurman
    Copy link
    Member Author

    Note: still need to update docs.

    @gvanrossum
    Copy link
    Member

    Yeah, I am generally in favor of this. Just yesterday there was a bug report (bpo-28070) where someone claimed that the flags from r'(ix)A' were incorrect. They were 96 and should be 98. (He was right, and it was fixed already.) The way he had to prove that was rather indirect. If the flags had printed like with this proposal it would have been much more straightforward.

    @rhettinger
    Copy link
    Contributor

    Ethan, can you give this class a better name than "Flags"? Perhaps something like "RegexFlags" or somesuch?

    @rhettinger rhettinger assigned ethanfurman and unassigned gvanrossum Sep 11, 2016
    @ethanfurman
    Copy link
    Member Author

    I did, immediately after your first post -- it's now RegexFlag. Thank you for the suggestion! Naming things can be hard, especially when trying to beat a deadline.

    @vstinner
    Copy link
    Member

    vstinner commented Nov 7, 2016

    The changeset 223731925d06 caused a performance regression: see issue bpo-28637.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 7, 2016

    New changeset d903a243c281 by Victor Stinner in branch '3.6':
    Issue bpo-28637: Revert issue bpo-28082, don't import enum in re
    https://hg.python.org/cpython/rev/d903a243c281

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 14, 2016

    New changeset 5fd69d4a93e0 by Victor Stinner in branch '3.6':
    Issue bpo-28637: Reapply changeset 223731925d06
    https://hg.python.org/cpython/rev/5fd69d4a93e0

    New changeset be66786e95de by Victor Stinner in branch '3.6':
    Issue bpo-28082: Add basic unit tests on re enums
    https://hg.python.org/cpython/rev/be66786e95de

    @vstinner
    Copy link
    Member

    The change 5fd69d4a93e0 (use IntFlag for re constants) made the "regex_compile" benchmark slower:

    Median +- std dev: [71c1970f27b6] 388 ms +- 3 ms -> [3cf248d10bed] 470 ms +- 4 ms: 1.21x slower

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 21, 2016

    New changeset 176fc21f8430 by Ethan Furman in branch '3.6':
    closes bpo-28082: doc update and NEWS entry
    https://hg.python.org/cpython/rev/176fc21f8430

    @python-dev python-dev mannequin closed this as completed Nov 21, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 21, 2016

    New changeset 493359386360 by Ethan Furman in branch '3.6':
    bpo-28082: actually include NEWS entry
    https://hg.python.org/cpython/rev/493359386360

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @merwok merwok mentioned this issue Feb 10, 2023
    Merged
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @ethanfurman ethanfurman

    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @vstinner @rhettinger @gvanrossum @serhiy-storchaka @ethanfurman