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.

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

Merged
merged 4 commits into from
Jun 10, 2017

Conversation

rowillia
Copy link
Contributor

@rowillia rowillia commented Jun 9, 2017

Running our unit tests with -bb enabled triggered this failure.

Running our unit tests with `-bb` enabled triggered this failure.
@jmphilli
Copy link

jmphilli commented Jun 9, 2017

+1

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a 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.

@@ -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
Copy link
Member

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)'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@rowillia
Copy link
Contributor Author

rowillia commented Jun 9, 2017

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
Copy link
Member

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.
Copy link
Member

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.

@rowillia
Copy link
Contributor Author

Thanks again @serhiy-storchaka for taking a look. I promise usually I spell my own name correctly 😆 !

@serhiy-storchaka serhiy-storchaka merged commit 171b9a3 into python:master Jun 10, 2017
@serhiy-storchaka
Copy link
Member

Thank you for your contribution @rowillia. Do you mind to backport this fix to 3.6?

@serhiy-storchaka serhiy-storchaka added needs backport to 3.6 type-bug An unexpected behavior, bug, or error labels Jun 10, 2017
@rowillia
Copy link
Contributor Author

Sounds good to me @serhiy-storchaka, thanks for your help landing this.

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Jun 15, 2017
…ed. (pythonGH-2016)

Running our unit tests with `-bb` enabled triggered this failure..
(cherry picked from commit 171b9a3)
serhiy-storchaka added a commit that referenced this pull request Jun 15, 2017
…ed. (GH-2016) (#2214)

Running our unit tests with `-bb` enabled triggered this failure..
(cherry picked from commit 171b9a3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants