8000 bpo-29869: Allow underscores in numeric literals in lib2to3. by nevsan · Pull Request #752 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

bpo-29869: Allow underscores in numeric literals in lib2to3. #752

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

Merged
merged 3 commits into from
Apr 13, 2017

Conversation

nevsan
Copy link
Contributor
@nevsan nevsan commented Mar 21, 2017

I found this while debugging google/yapf#370. The fix is quite simple and quick.

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:

  1. Sign the PSF contributor agreement. The "bugs.python.org username" requested by the form is the "Login name" field in "Your Details" at b.p.o
  2. Wait at least one US business day and then check the "Contributor form received entry under "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
  3. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@nevsan
Copy link
Contributor Author
nevsan commented Mar 21, 2017

I have completed the above steps.

@brettcannon
Copy link
Member

Could you open an issue for this on bugs.python.org, @nevsan and mention it in the title? (see the contributing guidelines for info.)

@brettcannon brettcannon added the type-bug An unexpected behavior, bug, or error label Mar 21, 2017
@nevsan nevsan changed the title Allow underscores in numeric literals in lib2to3. bpo-29869: Allow underscores in numeric literals in lib2to3. Mar 22, 2017
@nevsan nevsan force-pushed the underscores-in-numbers branch from b8581cb to 7d516a6 Compare March 22, 2017 14:55
@nevsan
Copy link
Contributor Author
nevsan commented Mar 22, 2017

The existing regular expressions weren't actually strict enough as is, so I made them even more correct. In particular, we must have at least one digit following 0[xXbBoO] and must be before any underscores.

I have a small set of test cases to examine correctness of these regular expressions: https://gist.github.com/nevsan/7fc78dc61d309842406d67d6839b9861

@nevsan nevsan force-pushed the underscores-in-numbers branch from 7d516a6 to ed419cc Compare March 23, 2017 04:16
@nevsan
Copy link
Contributor Author
nevsan commented Mar 23, 2017

Updated regular expressions to avoid catastrophic backtracking. Gist updated with new test cases (still passes).

@brettcannon
Copy link
8000
Member

@nevsan looks like Travis disagrees that the tests still pass 😉

@nevsan nevsan force-pushed the underscores-in-numbers branch 3 times, most recently from f7f52ef to 1304141 Compare March 23, 2017 19:41
@nevsan nevsan force-pushed the underscores-in-numbers branch from 1304141 to 2fd46cc Compare March 23, 2017 20:10
@nevsan
Copy link
Contributor Author
nevsan commented Mar 23, 2017

Took a few iterations, but I managed to fix up the regexes. I also added a test case to test numeric literals with underscores (and verified that it fails for the un-patched HEAD).

@rhettinger
Copy link
Contributor

LGTM

@brettcannon
Copy link
Member

@rhettinger would you mind officially registering your review approval? You can do that by going under "Files changed", clicking the green "Review changes", and then you can choose "Approve" and submit the review. Perk of this is your approval will show up under Reviewers so if another core dev has the time to merge and backport they know someone else already cleared the code.

@Mariatta
Copy link
Member

@nevsan Can you create the PR against the master branch?

8000

Mariatta added a commit that referenced this pull request Apr 13, 2017
…H-752)" (GH-1109)

This reverts commit 97a40b7.
The commit is supposed to go to the master branch first.
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
0