-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-30605: Fix compiling binary regexs with BytesWarnings enabled. #2016
Conversation
Running our unit tests with `-bb` enabled triggered this failure.
+1 |
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.
Add a test for bytes pattern, a Misc/NEWS entry, and your name in Misc/ACKS.
Lib/test/test_re.py
Outdated
@@ -1368,7 +1368,7 @@ def test_inline_flags(self): | |||
self.assertTrue(re.match(p, lower_char)) | |||
self.assertEqual( | |||
str(warns.warnings[0].message), | |||
'Flags not at the start of the expression %s' % p | |||
"Flags not at the start of the expression '%s'" % p |
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.
Just use %r
.
And add a test for bytes pattern b'A(?i)'
.
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.
Done
Thanks for your feedback @serhiy-storchaka ! Addressed your comments. |
Misc/ACKS
Outdated
@@ -1685,6 +1685,7 @@ Jakub Wilk | |||
Gerald S. Williams | |||
Jason Williams | |||
John Williams | |||
Roy Willams |
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.
Typo.
Misc/NEWS
Outdated
@@ -646,6 +646,9 @@ Library | |||
- bpo-29851: importlib.reload() now raises ModuleNotFoundError if the | |||
module lacks a spec. | |||
|
|||
- bpo-30605: re.compile() no longer raises a BytesWarning when compiling a | |||
bytes instance. |
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.
Add "Patch by Roy Williams."
"... when compiling a bytes instance with misplaced inline modifier." Or something like.
And usually new entries are added at the begin of a section, so they are listed in the reversed chronological order.
Thanks again @serhiy-storchaka for taking a look. I promise usually I spell my own name correctly 😆 ! |
Thank you for your contribution @rowillia. Do you mind to backport this fix to 3.6? |
Sounds good to me @serhiy-storchaka, thanks for your help landing this. |
…ed. (pythonGH-2016) Running our unit tests with `-bb` enabled triggered this failure.. (cherry picked from commit 171b9a3)
Running our unit tests with
-bb
enabled triggered this failure.