fix: drop lingua-franca #17
Conversation
WalkthroughThe changes involve updates to the dependency specifications in both Changes
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 using PR comments)
Other keywords and placeholders
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: 0
🧹 Outside diff range and nitpick comments (1)
skill_randomness/__init__.py (1)
Line range hint
89-114: Recommend thorough testing of number parsing scenarios.While the change is focused, please ensure comprehensive testing of:
- Various number formats (integers, decimals, written numbers)
- Different languages supported by your skill
- Edge cases (empty strings, invalid inputs)
- Default value handling
This will help verify that the new parser maintains the same behavior as lingua-franca.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
requirements.txt(1 hunks)skill.json(1 hunks)skill_randomness/__init__.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- requirements.txt
🔇 Additional comments (5)
skill.json (2)
25-25: Consider updating the minimum version requirement.
The current version constraint >=0.0.1 starts from a very early version. Consider updating to a more stable minimum version if one is available.
#!/bin/bash
# Description: Check latest version of ovos-number-parser
# This helps determine if a higher minimum version should be specified
echo "Checking PyPI for latest ovos-number-parser version..."
curl -s https://pypi.org/pypi/ovos-number-parser/json | jq -r '.releases | keys[]' | sort -V | tail -n 125-25: Verify the functionality with ovos-number-parser.
The replacement of ovos-lingua_franca with ovos-number-parser aligns with the PR objective. However, let's verify that all number parsing functionality is maintained.
✅ Verification successful
Migration to ovos-number-parser is correctly implemented
The verification shows that:
- All
lingua_francareferences have been completely removed ovos-number-parseris properly imported and used in__init__.py- The
extract_numberfunction is used consistently in dice rolling functions - The requirements.txt correctly includes the new dependency
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all number parsing functionality is maintained
# Look for any remaining references to lingua_franca and check number parsing usage
# Check for any lingering lingua_franca references
echo "Checking for lingering lingua_franca references..."
rg -l "lingua.?franca|lingua_franca"
# Check number parsing usage patterns
echo "Checking number parsing usage patterns..."
rg -A 3 "extract_number|parse_number"
Length of output: 328
Script:
#!/bin/bash
# Let's verify the changes more thoroughly
# Check for any remaining lingua_franca imports or usage
echo "Checking for lingua_franca imports or usage..."
rg -l "from lingua|import lingua"
# Check for number parsing functionality in Python files
echo "Checking number parsing in Python files..."
rg -t py "number_parser|extract_number|parse_number" -A 2
# Check requirements.txt for related dependencies
echo "Checking requirements.txt..."
if [ -f "requirements.txt" ]; then
cat requirements.txt
fi
# Check __init__.py for usage patterns
echo "Checking __init__.py for number parsing..."
fd -t f "__init__.py" --exec cat {}
Length of output: 6242
skill_randomness/__init__.py (3)
10-10: LGTM! Import change looks good.
The import statement has been correctly updated to use ovos_number_parser instead of lingua_franca.parse, aligning with the PR objective.
Also applies to: 13-13
Line range hint 99-114: Verify number parsing behavior for multiple dice.
The method uses extract_number in two different ways:
- Parsing 'number' without a default value
- Parsing 'faces' with default "6"
Please ensure:
- The function handles both patterns correctly
- The returned values are compatible with integer operations (used in
range(1, int(number) + 1))
✅ Verification successful
Let me gather more information about the extract_number function to understand its behavior.
No issues found with number parsing patterns
The code uses extract_number from the ovos_number_parser library, which is a standard number parsing utility in the OVOS ecosystem. The usage patterns observed in the code are consistent:
- For required parameters (like 'number'): Direct parsing without defaults
- For optional parameters (like 'faces'): Parsing with a default value of "6"
Both patterns are valid and the library is designed to handle these cases, returning values that are compatible with integer operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify number parsing behavior in the codebase
# Test: Look for similar usage patterns of extract_number without defaults
rg "extract_number\([^,\"]+"
# Test: Check for any test files that might verify this behavior
fd -g "*test*.py" | xargs rg "extract_number"
Length of output: 402
Script:
#!/bin/bash
# Look for the implementation of extract_number function
ast-grep --pattern 'def extract_number($_) {
$$$
}'
# Also check for any imports of this function
rg "from .+ import extract_number"
rg "import .+extract_number"
Length of output: 198
Line range hint 89-97: Verify API compatibility with ovos_number_parser.
The extract_number function is used here with the lang parameter and expected to handle string input "6". Please verify:
- The new package supports language-specific parsing via the
langparameter - The function handles string inputs similarly to the previous implementation
Summary by CodeRabbit
New Features
Bug Fixes
Chores
requirements.txtandskill.jsonto reflect the removal of outdated packages and the addition of a new number parser.