Repository navigation
Fix float parsing - #5643
Merged
Merged
Fix float parsing#5643
Conversation
coolreader18
force-pushed
the
fix-float-parsing
branch
from
March 31, 2025 02:11
8fb5c87 to
14e1013
Compare
coolreader18
force-pushed
the
fix-float-parsing
branch
from
March 31, 2025 02:22
14e1013 to
d2a2120
Compare
youknowone
requested changes
Mar 31, 2025
| if re.is_sign_positive() { | ||
| return im_part; | ||
| } else { | ||
| re.to_string() |
Member
There was a problem hiding this comment.
Suggested change
| re.to_string() | |
| "0" |
or
Suggested change
| re.to_string() | |
| "0.0" |
?
Calling specifically f64::to_string() only for zero and float::to_string(re) for non-zero looks like a bug or unnecessary
Member
Author
There was a problem hiding this comment.
This was copied over from the old implementation, but I agree.
youknowone
approved these changes
Mar 31, 2025
zzarbttoo
added a commit
to zzarbttoo/RustPython
that referenced
this pull request
Sep 20, 2026
`char_to_decimal` carried its own sorted table of 590 decimal digit characters, added in RustPython#5643 because `common` had no way to reach the Unicode numeric data at the time: it lived in `stdlib`, which sits above `common` in the dependency graph. The table's own comment said as much — "this should probably live in a crate like unic or unicode-properties". RustPython#8211 extracted that data into `rustpython-unicode`, below `common`, and made `common` depend on it, so the table can now be dropped in favour of `Ucd::decimal` — the same source `unicodedata.decimal` consults. Every one of the 590 characters keeps its previous value, so this is not a behaviour change for anything the table already covered. It does pick up the 180 decimal digits from 17 scripts added to Unicode after the table was written (Hanifi Rohingya, Wancho, Tangsa, segmented digits and others), and it removes the possibility of `char_to_decimal` and `unicodedata.decimal` disagreeing. The lookup costs about 3ns more per non-ASCII character, since it reads a property trie and a value table instead of one 2.4KB array. ASCII input never reaches it — the sole caller, `transform_decimal_and_space_to_ascii`, returns early for ASCII strings — so ordinary numeric parsing is unaffected. Assisted-by: Claude Code:claude-opus-5
1 of 2 tasks
zzarbttoo
added a commit
to zzarbttoo/RustPython
that referenced
this pull request
Sep 20, 2026
`char_to_decimal` carried its own sorted table of 590 decimal digit characters, added in RustPython#5643 because `common` had no way to reach the Unicode numeric data at the time: it lived in `stdlib`, which sits above `common` in the dependency graph. The table's own comment said as much — "this should probably live in a crate like unic or unicode-properties". RustPython#8211 extracted that data into `rustpython-unicode`, below `common`, and made `common` depend on it, so the table can now be dropped in favour of `Ucd::decimal` — the same source `unicodedata.decimal` consults. Every one of the 590 characters keeps its previous value, so this is not a behaviour change for anything the table already covered. It does pick up the 180 decimal digits from 17 scripts added to Unicode after the table was written (Hanifi Rohingya, Wancho, Tangsa, segmented digits and others), and it removes the possibility of `char_to_decimal` and `unicodedata.decimal` disagreeing. The lookup costs about 3ns more per non-ASCII character, since it reads a property trie and a value table instead of one 2.4KB array. ASCII input never reaches it — the sole caller, `transform_decimal_and_space_to_ascii`, returns early for ASCII strings — so ordinary numeric parsing is unaffected. Assisted-by: Claude Code:claude-opus-5
zzarbttoo
added a commit
to zzarbttoo/RustPython
that referenced
this pull request
Sep 20, 2026
`char_to_decimal` carried its own sorted table of 590 decimal digit characters, added in RustPython#5643 because `common` had no way to reach the Unicode numeric data at the time: it lived in `stdlib`, which sits above `common` in the dependency graph. The table's own comment said as much — "this should probably live in a crate like unic or unicode-properties". RustPython#8211 extracted that data into `rustpython-unicode`, below `common`, and made `common` depend on it, so the table can now be dropped in favour of `Ucd::decimal` — the same source `unicodedata.decimal` consults. This fixes a CPython incompatibility. The table was last updated for Unicode 10; the bundled UCD is Unicode 17, which has 180 more decimal digits across 18 scripts (Hanifi Rohingya, Wancho, Tangsa, Ol Onal, segmented digits and others). Those were rejected by `int()` and `float()` even though `unicodedata.decimal` knew their value: >>> int('\u{10D33}') # HANIFI ROHINGYA DIGIT THREE ValueError # CPython: 3 >>> unicodedata.decimal('\u{10D33}') 3 All 590 characters the table covered keep their previous value, so nothing that already worked changes, and the two lookups can no longer drift apart on a future Unicode update. The lookup costs about 1.5ns more per non-ASCII character, since it reads a property trie and a value table instead of binary-searching one 2.4KB array: `int()` over a 4000-digit non-ASCII string goes from 12.2 to 13.6 ns/char. ASCII input never reaches it — the sole caller, `transform_decimal_and_space_to_ascii`, returns early for ASCII strings — so ordinary numeric parsing is unaffected. Assisted-by: Claude Code:claude-opus-5
youknowone
pushed a commit
that referenced
this pull request
Sep 20, 2026
`char_to_decimal` carried its own sorted table of 590 decimal digit characters, added in #5643 because `common` had no way to reach the Unicode numeric data at the time: it lived in `stdlib`, which sits above `common` in the dependency graph. The table's own comment said as much — "this should probably live in a crate like unic or unicode-properties". #8211 extracted that data into `rustpython-unicode`, below `common`, and made `common` depend on it, so the table can now be dropped in favour of `Ucd::decimal` — the same source `unicodedata.decimal` consults. This fixes a CPython incompatibility. The table was last updated for Unicode 10; the bundled UCD is Unicode 17, which has 180 more decimal digits across 18 scripts (Hanifi Rohingya, Wancho, Tangsa, Ol Onal, segmented digits and others). Those were rejected by `int()` and `float()` even though `unicodedata.decimal` knew their value: >>> int('\u{10D33}') # HANIFI ROHINGYA DIGIT THREE ValueError # CPython: 3 >>> unicodedata.decimal('\u{10D33}') 3 All 590 characters the table covered keep their previous value, so nothing that already worked changes, and the two lookups can no longer drift apart on a future Unicode update. The lookup costs about 1.5ns more per non-ASCII character, since it reads a property trie and a value table instead of binary-searching one 2.4KB array: `int()` over a 4000-digit non-ASCII string goes from 12.2 to 13.6 ns/char. ASCII input never reaches it — the sole caller, `transform_decimal_and_space_to_ascii`, returns early for ASCII strings — so ordinary numeric parsing is unaffected. Assisted-by: Claude Code:claude-opus-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.