-
Notifications
You must be signed in to change notification settings - Fork 0
ISSUE - 2 & 3 | Fixed the Conversion Issue in nepali Number System #9
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
Conversation
WalkthroughThis pull request makes three key updates. The Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant W as word_to_num()
participant N as number_formation()
participant D as get_decimal_sum()
U->>W: Provide number words input
W->>N: Process integer part and validate order
N->>W: Return integer value (or error)
alt Includes decimal point
W->>D: Validate decimal words and compute sum
D->>W: Return decimal value (or error)
end
W-->>U: Return final numeric result or error message
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
WordToNumber/word_to_number.py:346
- In the block handling the 'thousand' denomination, the computed 'hundreds' value is assigned but not added to total_sum. To fix this, add 'total_sum += hundreds' after the calculation.
if thousand_index > -1 and thousand_index != len(clean_numbers)-1:
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
WordToNumber/word_to_number.py (2)
79-82: Consider raising an error if no valid words are found, instead of returning 0.Silently returning 0 might mask incorrect usage. A more explicit approach (e.g., a clear error) can help catch mistakes quickly.
if not numbers: - return 0 + raise ValueError("No valid words provided for number formation.")
372-376: Raise exceptions with “from e” to retain original traceback context.For instance:
except ValueError as e: - raise ValueError(f"Error processing decimal part of '{number_sentence}': {str(e)} ...") + raise ValueError(f"Error processing decimal part of '{number_sentence}': {str(e)} ...") from eThis approach clarifies that the original or preceding error triggered the new exception.
🧰 Tools
🪛 Ruff (0.8.2)
376-376: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)WordToNumber/word_to_number.py(5 hunks)setup.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
WordToNumber/word_to_number.py
376-376: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (12)
.gitignore (1)
6-7: Good addition to ignore test bytecode.Adding
/tests/__pycache__ensures that test bytecode artifacts remain untracked, keeping the repository clean.WordToNumber/word_to_number.py (10)
3-7: Great addition of a clear module-level docstring.Providing an overview at the top of the module helps users quickly understand the scope and functionalities, especially for mixed or Nepali number systems.
53-71: Excellent function docstring.This comprehensive explanation enhances clarity for
number_formation, including usage examples and argument details.
96-107: Docstring improvements look great.The explanation of valid decimal digits and potential exceptions will help developers better handle unusual decimal inputs.
128-137: Module-level clarity appreciated.Highlighting complex phrases, decimal handling, and detailed error messages helps readers understand what to expect from
word_to_num.
139-145: Detailed argument and return documentation is valuable.Stating exact exception conditions is especially important for robust number-parsing libraries.
183-190: Redundant terms check is a solid addition.Catching repeated denominations is crucial to avoid ambiguous parses. Great job providing a clear error message.
217-238: Impressive ordering checks.Enforcing the correct sequence of billion/million/thousand/arba/crore/lakh to prevent ill-formed inputs is a robust validation step.
241-256: Mixing system constraints are well-handled.Raising a descriptive
ValueErrorwhen mixing American and Indian systems will reduce user confusion.
259-272: Good check for missing intermediate terms.Ensuring the correct hierarchy helps maintain logical consistency in large numeric phrases.
345-365: Thoughtful handling of Indian system partial phrases.Covering scenarios when certain denominations are missing or appear at the end fosters a more flexible user experience.
setup.py (1)
5-5: Version bump is consistent with added functionality.Using
1.1.0appropriately signals new features and enhancements in error handling for Nepali number conversions.
Docstrings generation was requested by @santoshvandari. * #9 (comment) The following files were modified: * `WordToNumber/word_to_number.py`
|
Note Generated docstrings for this pull request at #10 |
This pull request includes significant improvements and enhancements to the
WordToNumbermodule, focusing on better documentation, error handling, and validation for converting textual number representations to numeric values. The most important changes include the addition of detailed docstrings, enhanced error handling, and improved validation for number word order and redundancy.Documentation Improvements:
WordToNumber/word_to_number.py: Added detailed module-level and function-level docstrings to improve code documentation and clarity. [1] [2] [3]Error Handling Enhancements:
WordToNumber/word_to_number.py: Improved error messages for invalid inputs, redundant number words, and incorrect number word order, providing specific feedback to the user. [1] [2]Validation Improvements:
WordToNumber/word_to_number.py: Added checks for invalid number words, redundant terms, and mixed number systems, ensuring consistent and correct number word usage. [1] [2]Version Update:
setup.py: Updated the version of theWordToNumberpackage to1.1.0to reflect the new enhancements and improvements.#2
#3
Summary by CodeRabbit