-
-
Notifications
You must be signed in to change notification settings - Fork 110
Enhance number to words #227
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
base: main
Are you sure you want to change the base?
Conversation
| # Handle scientific notation conversion | ||
| if isinstance(num, float): | ||
| formatted = f"{num:.17g}" | ||
| if 'e' in formatted.lower(): | ||
| num = f"{num:.17f}".rstrip('0').rstrip('.') | ||
| else: | ||
| num = formatted | ||
| else: | ||
| num = str(num) |
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'd recommend to extract this logic into a helper function/method, something like num = _normalize_number(num). That way, the comment can be in the docstring. This approach also limits the cyclomatic complexity.
jaraco
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.
Looking good! Can you add a test capturing the new expectation?
|
Unfortunately due to my novice programming experience I don't know what "capturing the new expectation" means...Not joking sadly. |
Understood. What I mean is that this change introduces a "new expectation" that inflect now handles numbers in scientific notation. I want to add a test that will fail on the current code in main but pass after applying the proposed changes. The tests in this project can exist in a number of places (probably in this order of preference where appropriate):
To get started, you'll want to first validate that you can run the tests on You're also welcome to join my Discord server for interactive support if that might help. |
|
Are you able to finish this pull request by chance? Unfortunately I can't even though I appreciate you assigning the issue to me. |
Should resolve the issue located at #226:
This PR addresses an issue where Python's scientific notation handling leads to incorrect number-to-word conversions. Currently, when converting small numbers like 0.000001, Python represents them in scientific notation (1e-06), causing inflect to incorrectly interpret them as different numbers (e.g., interpreting 1e-06 as 106).
The proposed solution adds scientific notation handling to the
number_to_wordsmethod:This modification:
The solution handles all common cases.
Here is a simple test script:
SCRIPT
What is ".17g"?
The
.17gin the format specifier(f"{num:.17g}")means:17: Use up to 17 significant digitsg: Use either fixed-point or scientific notation, whichever is more appropriate for the number's sizeThe
gformat (general format) is smart and automatically chooses between:fformat (fixed-point) for numbers close to 1eformat (scientific) for very large or very small numbersWe use
.17gfirst to detect if scientific notation is being used (by checking for 'e'), and if it is, we then force fixed-point notation with.17fto get the full decimal representation.How does
num2wordsdo it?After making this pull request I checked whether the
num2wordsalready handles it and they do. Here's a summary for a potential future expansion, but this pull request should address the immediate issue:Summary of their approach:
Base Processing (
base.py)float2tupleandto_cardinal_floatmethods for number handlingDecimal(str(value)).as_tuple().exponentfor precise decimal handlingLanguage Support (
lang_EN.py)Key Advantages
The Decimal-based approach offers several benefits:
Comparison
While more mathematically rigorous than string formatting, this approach would require significant architectural changes to implement in
inflect.