-
-
Notifications
You must be signed in to change notification settings - Fork 634
Make preparser can handle large(4301+ digits) integers #40178
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
Changes from 5 commits
45aaaa9
49ff108
aaf84e1
f43e339
50c9eb1
de3a3da
ee21b0c
c6d9e2d
94b8909
266a113
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1124,8 +1124,9 @@ | |||||
name-construction pairs | ||||||
|
||||||
- ``quotes`` -- string (default: ``"'"``); used to surround string | ||||||
arguments to RealNumber and ComplexNumber. If ``None``, will rebuild | ||||||
the string using a list of its Unicode code-points. | ||||||
arguments to RealNumber and ComplexNumber, and Integer when the | ||||||
number is longer than 4300 digits. If ``None``, will rebuild the | ||||||
string using a list of its Unicode code-points. | ||||||
|
||||||
OUTPUT: | ||||||
|
||||||
|
@@ -1189,6 +1190,14 @@ | |||||
sage: preparse_numeric_literals('000042') | ||||||
'Integer(42)' | ||||||
|
||||||
Cheeck that :issue:`40179` is fixed:: | ||||||
sage: preparse_numeric_literals("1" * 4300) == f"Integer({"1" * 4300})" | ||||||
Check failure on line 1194 in src/sage/repl/preparse.py
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This test is broken because the inner quote on the RHS needs to be different (or escaped There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interestingly it's new feature in 3.12 https://stackoverflow.com/a/77858818/5267751 . Lowest supported version is (still) 3.11 so yes. |
||||||
True | ||||||
sage: preparse_numeric_literals("1" * 4301) == f"Integer({"1" * 4301})" | ||||||
Check failure on line 1196 in src/sage/repl/preparse.py
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Same as above. |
||||||
False | ||||||
sage: preparse_numeric_literals("1" * 4301) == f"Integer('{"1" * 4301}')" | ||||||
Check failure on line 1198 in src/sage/repl/preparse.py
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this will work (the test is a bit different). Please check locally before pushing. |
||||||
True | ||||||
|
||||||
Test underscores as digit separators (PEP 515, | ||||||
https://www.python.org/dev/peps/pep-0515/):: | ||||||
|
||||||
|
@@ -1253,6 +1262,10 @@ | |||||
'RealNumber(str().join(map(chr, [51, 46, 49, 52])))' | ||||||
sage: preparse_numeric_literals('5j', quotes=None) | ||||||
'ComplexNumber(0, str().join(map(chr, [53])))' | ||||||
sage: preparse_numeric_literals("1" * 4301, quotes=None) == f'Integer(str().join(map(chr, {[49] * 4301})))' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests that specifically address a bug should provide a link to the bug they are testing a fix for. Add a brief explanation of what this test is doing before hand and use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree; in particular, this should be moved up to the place with all of the other tests related to this issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @soon-haari My suggestion was to move the test, not add another docstring. I think it is better if they are all grouped together. |
||||||
True | ||||||
sage: preparse_numeric_literals("1" * 4301, quotes="'''") == f"Integer('''{"1" * 4301}''')" | ||||||
Check failure on line 1267 in src/sage/repl/preparse.py
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this test has become a bit overkill now. I would consider removing it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! Althought my local test using |
||||||
True | ||||||
""" | ||||||
literals = {} | ||||||
last = 0 | ||||||
|
@@ -1324,7 +1337,13 @@ | |||||
# Python 3 does not allow leading zeroes. Sage does, so just strip them out. | ||||||
# The number is still interpreted as decimal, not octal! | ||||||
num = re.sub(r'^0+', '', num) | ||||||
num_make = "Integer(%s)" % num | ||||||
if len(num) <= 4300: | ||||||
num_make = "Integer(%s)" % num | ||||||
elif quotes: | ||||||
num_make = "Integer(%s%s%s)" % (quotes, num, quotes) | ||||||
else: | ||||||
code_points = list(map(ord, list(num))) | ||||||
num_make = "Integer(str().join(map(chr, %s)))" % code_points | ||||||
|
||||||
literals[num_name] = num_make | ||||||
|
||||||
|
Uh oh!
There was an error while loading. Please reload this page.