-
-
Notifications
You must be signed in to change notification settings - Fork 53
Implement underscores in numeric literals #21
Conversation
|
@ddfisher Fixed the bug with fractional part, sorry for not noticing it earlier. |
|
Sorry for being slow to review this! I was hoping to get to it this week but had some other things take precedence. It's at the top of my list for next week! Edit: Nevermind! I found time this evening. |
ddfisher
left a comment
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.
Thanks! This PR looks great overall -- I just have one minor nitpick.
It was difficult for me to evaluate your tokenizer changes, even when comparing them to the commit to CPython (because that change also introduced a bunch of unrelated open and close braces). I ran this on a large codebase and didn't see any problems, so I'm going to assume you've made equivalent changes.
ast35/Parser/Python.asdl
Outdated
| | Compare(expr left, cmpop* ops, expr* comparators) | ||
| | Call(expr func, expr* args, keyword* keywords) | ||
| | Num(object n) -- a number as a PyObject. | ||
| | Num(object n, int? underscores) -- a number as a PyObject. |
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 think it'd be a bit clearer if this was named contains_underscores instead. Also, could you add a comment noting that this field is not part of standard Python and its purpose is to signal that a newer Python version feature was used?
|
@ddfisher Thank you for review!
I used both positive and negative example numeric literals from original Python |
|
Great, thanks! |
FWIW, here is the implementation of underscores in numeric literals. I just cherry-picked the necessary parts from the original implementation in Python 3.6 by Georg and Serhiy. This could be useful for mypy.
@ddfisher Please take a look.