bpo-31369: include RegexFlag in re.all by akulakov · Pull Request #30279 · python/cpython (original) (raw)

akulakov

JelleZijlstra

@@ -710,6 +713,9 @@ form.
string and immediately before the newline (if any) at the end of the string.
Corresponds to the inline flag ``(?m)``.
.. data:: NOFLAG
Indicates no flag being applied, the value is ``0``.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, explain when this is useful and add versionadded.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be used for example as a default value for a function parameter, and also as a base value that will later conditionally be ORed with other flags. But this explanation would be confusing to many readers without code examples.

An example as follows may be enough:

def myfunc(text, flag=re.NOFLAG):
    return re.match(text, flag)

What do you think?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think that's a useful example.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When was it added? Edit: found #72269 this added the enum, this PR here actually added NOFLAG despite its title

It seems that the discussion in #57594 was ignored, plus the naming is slightly stange (no flags reads better than no flag)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not ignored, missed. It took place 11 years prior. We can certainly add a NOFLAGS name now, and make NOFLAG be an alias.

@@ -137,14 +137,15 @@
"findall", "finditer", "compile", "purge", "template", "escape",
"error", "Pattern", "Match", "A", "I", "L", "M", "S", "X", "U",
"ASCII", "IGNORECASE", "LOCALE", "MULTILINE", "DOTALL", "VERBOSE",
"UNICODE",
"UNICODE", "RegexFlag",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also add NOFLAG.

@akulakov

@akulakov

@akulakov @JelleZijlstra

Co-authored-by: Jelle Zijlstra jelle.zijlstra@gmail.com

@JelleZijlstra

@ethanfurman would you like to look at this as it's enum-related?

@bdice

Thanks! I was just looking into this and was puzzled why a NOFLAG (or similar) object didn't exist. Happy to see this PR. 😄

On a related note, I noticed that the type of re.compile("").flags is int and not re.RegexFlag as I expected. I might file an issue for this.

ethanfurman

@@ -637,6 +637,11 @@ form.
programs that use only a few regular expressions at a time needn't worry
about compiling regular expressions.
.. class:: RegexFlag
An :class:`~enum.Enum` class containing the regex options listed below.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the tilde in ~enum.Enum mean?

Also, it should be enum.IntFlag.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tilde creates a link with only the last element visible, i.e. Enum here, but since it should be IntFlag, I also removed the tilde as IntFlag is less known (though reader can still hover over shortened name to see where it links of course).

An :class:`~enum.Enum` class containing the regex options listed below.
.. versionadded:: 3.6 (documented in 3.11)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps .. versionchanged:: added to all in 3.11

@bedevere-bot

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@akulakov

ethanfurman

.. versionadded:: 3.6 (documented in 3.11)
.. versionadded:: added to ``__all__`` in 3.11)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a parenthesis, so lets change to:

.. versionadded:: 3.11 - added to ``__all__``

@clinton21obeng

Stop

On Sat, Feb 5, 2022, 12:03 AM Ethan Furman ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In Doc/library/re.rst <#30279 (comment)>: > - .. versionadded:: 3.6 (documented in 3.11) + .. versionadded:: added to ``__all__`` in 3.11) Missing a parenthesis, so lets change to: .. versionadded:: 3.11 - added to ``__all__`` — Reply to this email directly, view it on GitHub <#30279 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AV5LH7EJ7NIFS3KIFCKMXF3UZRSNDANCNFSM5K3D7JFQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

@akulakov

ethanfurman

@akulakov

@akulakov akulakov deleted the 31369-Add-RegexFlag-to-__all__ branch

February 5, 2022 03:57