-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
In Doc/library/re.rst rename a variable from match to m
#123003
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.
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
base: main
Are you sure you want to change the base?
Conversation
Since version 3.10 `match` has been a soft keyword, so examples ought to not use it as a variable name. https://docs.python.org/3/reference/lexical_analysis.html#soft-keywords The new name is `m`, that used for match objects in the other examples in re.rst.
|
Nevermind, you already observed it. |
| m = re.search(pattern, string) | ||
| if m: | ||
| process(m) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To promote good practice, we could suggest
| m = re.search(pattern, string) | |
| if m: | |
| process(m) | |
| m = re.search(pattern, string) | |
| if m is not None: | |
| process(m) |
If we want to suggest using assignment expressions, we could also use:
| m = re.search(pattern, string) | |
| if m: | |
| process(m) | |
| if m := re.search(pattern, string): | |
| process(m) |
Or both! (I probably wouldn't go for this...)
| m = re.search(pattern, string) | |
| if m: | |
| process(m) | |
| if (m := re.search(pattern, string)) is not None: | |
| process(m) |
A
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having is not None does not seem to me better practice. There are two possibly relevant parts of PEP 8:
Also, beware of writing
if xwhen you really meanif x is not None– e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!
and
For sequences, (strings, lists, tuples), use the fact that empty sequences are false:
# Correct: if not seq: if seq:# Wrong: if len(seq): if not len(seq):
Now, the first does not apply here, because, as the documentation immediately before the example says,
Match objects always have a boolean value of
True. Since match() and search() returnNonewhen there is no match, you can test whether there was a match with a simpleifstatement [Emphasis added]
On the other hand, the spirit of the second excerpt from PEP 8 argues for availing of truthiness and falsiness and thus against the redundant is not None.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have a strong opinion on whether to use the assignment operator here. (I myself would not, reserving it for cases like
[process(m) for pattern in patterns if (m := re.search(pattern, string)]and
if ...:
...
elif (m := re.search(pattern, string):
process(m)).
| Match objects always have a boolean value of ``True``. | ||
| Since :meth:`~Pattern.match` and :meth:`~Pattern.search` return ``None`` | ||
| when there is no match, you can test whether there was a match with a simple | ||
| ``if`` statement:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ``if`` statement:: | |
| ``if ... is not None`` statement:: |
I agree with Adam that the example should be updated, but the text should be updated accordingly.
Since version 3.10
matchhas been a soft keyword, so examples ought to not useit as a variable name.
https://docs.python.org/3/reference/lexical_analysis.html#soft-keywords
The new name is
m, that used for match objects in the other examples inre.rst.
📚 Documentation preview 📚: https://cpython-previews--123003.org.readthedocs.build/