Feat/pronounce digits #41
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #41 +/- ##
=====================================
Coverage ? 0.00%
=====================================
Files ? 65
Lines ? 16419
Branches ? 0
=====================================
Hits ? 0
Misses ? 16419
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| return op_val + " point " + decimal_part | ||
| return op_val | ||
|
|
||
| if "." in str(number): |
There was a problem hiding this comment.
Any reason we treat non-str numbers differently here instead of just starting with number = str(number)?
| self.assertEqual(pronounce_digits(10.999999), "ten point nine nine") | ||
| self.assertEqual(pronounce_digits(10.999999, places=0), "ten") |
There was a problem hiding this comment.
These aren't correctly rounded; I think this should either treat places as "significant figures", or specifically note in the docstring that places are not sig figs and decimals are truncated as opposed to rounded
There was a problem hiding this comment.
maybe an extra flag for rounding or not defaulting to True?
There was a problem hiding this comment.
I suspect its parsing as a string instead of using round()
There was a problem hiding this comment.
The method is a formating method should not modify input, imho not rounding is the expected behaviour and rounding should be done before calling method if needed or via optional flag. This is reading digits not pronouncing the number, the rounding feels application specific even if I can't come up with a good use case where we don't want it....
|
@coderabbitai review |
Actions performedReview triggered.
|
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 3 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new function Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant format.py
participant format_en.py
Caller->>+format.py: pronounce_digits(number, places, all_digits, lang)
format.py->>+format_en.py: pronounce_digits_en(number, places, all_digits)
format_en.py-->>-format.py: Pronounced digits string
format.py-->>-Caller: Pronounced digits string
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- lingua_franca/format.py (2 hunks)
- lingua_franca/lang/format_en.py (1 hunks)
- test/unittests/test_format_en.py (2 hunks)
Additional comments not posted (2)
lingua_franca/format.py (1)
37-37: Function registration is correct.The
pronounce_digitsfunction has been correctly registered in the_REGISTERED_FUNCTIONStuple. This is necessary for the function to be accessible as intended.test/unittests/test_format_en.py (1)
39-39: Import statement updated correctly.The import statement has been correctly updated to include
pronounce_digits. This is necessary for the tests to access the newly added function.
| def pronounce_digits_en(number, places=2, all_digits=False): | ||
| if isinstance(number, str): | ||
| op_val = number | ||
| decimal_part = "" | ||
| if "." in number: | ||
| op_val, decimal_part = number.split(".") | ||
| if all_digits: | ||
| op_val = " ".join([pronounce_number_en(int(ch)) | ||
| for ch in op_val]) | ||
| else: | ||
|
|
||
| op_val = pronounce_number_en(int(op_val)) | ||
| if decimal_part: | ||
| decimal_part = " ".join([pronounce_number_en(int(ch)) | ||
| for ch in decimal_part[:places]]) | ||
| return op_val + " point " + decimal_part | ||
| return op_val | ||
|
|
||
| if "." in str(number): | ||
| op_val, decimal_part = str(number).split(".") | ||
| decimal_part = " ".join([pronounce_number_en(int(ch)) | ||
| for ch in decimal_part[:places]]) | ||
| else: | ||
| op_val = str(number) | ||
| decimal_part = "" | ||
|
|
||
| if all_digits: | ||
| op_val = " ".join([pronounce_number_en(int(ch)) | ||
| for ch in op_val]) | ||
| else: | ||
| op_val = pronounce_number_en(int(op_val)) | ||
| if decimal_part: | ||
| return op_val + " point " + decimal_part | ||
| return op_val | ||
|
|
There was a problem hiding this comment.
Review of pronounce_digits_en function
The function pronounce_digits_en has been added to handle the pronunciation of digits. It allows for specifying the number of decimal places and whether to pronounce all digits. Here are a few observations and suggestions:
- Type Checking: The use of
isinstanceto differentiate behavior based on the type ofnumberis a good practice. This ensures that the function can handle different types of inputs robustly. - Handling of Decimal Parts: The logic to split and handle the decimal part of the number is clear and well-implemented. However, consider adding error handling for cases where the input is not as expected (e.g., non-numeric strings).
- Use of List Comprehensions: The use of list comprehensions for constructing the spoken form of the number is efficient and Pythonic.
- Potential Refactoring: The blocks of code handling the string and non-string inputs are quite similar and could potentially be refactored to reduce duplication. This could improve maintainability.
- Documentation: Ensure that the function's docstring is updated to reflect all parameters and possible edge cases.
Overall, the function implementation looks solid but consider the above points to further enhance the code quality and robustness.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
port MycroftAI#150
Summary by CodeRabbit
New Features
Tests