fix: drop lingua-franca #85
Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 27 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. WalkthroughThe changes include significant updates to the weather skill's codebase, primarily involving the transition from the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WeatherIntent
participant UtteranceNormalizerPlugin
participant WeatherReport
User->>WeatherIntent: Send utterance
WeatherIntent->>UtteranceNormalizerPlugin: Normalize utterance
UtteranceNormalizerPlugin->>WeatherIntent: Return normalized utterance
WeatherIntent->>WeatherReport: Process normalized utterance
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 (2)
weather_helpers/dialog.py (2)
37-37: Remove unused import.The
Optionaltype hint is imported but never used in the code.-from typing import List, Optional +from typing import List🧰 Tools
🪛 Ruff
37-37:
typing.Optionalimported but unusedRemove unused import:
typing.Optional(F401)
Line range hint
434-449: Add return type hint for better type safety.The new function would benefit from a return type hint.
-def get_dialog_for_timeframe(intent_data: WeatherIntent, - weather: WeatherReport): +def get_dialog_for_timeframe(intent_data: WeatherIntent, + weather: WeatherReport) -> WeatherDialog:🧰 Tools
🪛 Ruff
37-37:
typing.Optionalimported but unusedRemove unused import:
typing.Optional(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
__init__.py(3 hunks)requirements.txt(1 hunks)weather_helpers/dialog.py(5 hunks)weather_helpers/intent.py(2 hunks)weather_helpers/util.py(2 hunks)
🧰 Additional context used
🪛 Ruff
weather_helpers/dialog.py
37-37: typing.Optional imported but unused
Remove unused import: typing.Optional
(F401)
🔇 Additional comments (12)
requirements.txt (2)
5-7: LGTM: New dependencies align with migration strategy.
The addition of ovos-number-parser, ovos-date-parser, and ovos-utterance-normalizer as replacements for lingua-franca is appropriate. However, note that these packages are in early development (<1.0.0), which means they might introduce breaking changes in future updates.
#!/bin/bash
# Verify that lingua-franca is completely removed
rg -l "lingua.?franca|lingua_franca"4-4: Caution: Using alpha version of ovos-workshop.
The dependency on an alpha version (>=2.2.0a1) of ovos-workshop could introduce instability. Consider pinning to a stable version if available.
✅ Verification successful
Alpha version of ovos-workshop is acceptable in this context
The codebase shows extensive integration with ovos-workshop's core functionality (IntentBuilder, OVOSSkill, decorators, etc.). The version constraint >=2.2.0a1,<3.0.0 is appropriate because:
- The skill is part of the OVOS ecosystem and follows its development cycle
- The upper bound
<3.0.0prevents major breaking changes - All imports are from stable APIs that are unlikely to change in alpha releases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check available versions of ovos-workshop
curl -s https://pypi.org/pypi/ovos-workshop/json | jq -r '.releases | keys[]' | sort -V | tail -n 5
Length of output: 178
Script:
#!/bin/bash
# Let's try a different approach to verify the migration from lingua-franca
# and check the stability of ovos-workshop
# 1. Search for any remaining lingua-franca references
rg -i "lingua.?franca|lingua_franca"
# 2. Check if ovos-workshop is imported and how it's used
rg -A 2 "from ovos.workshop|import ovos.workshop|from ovos_workshop|import ovos_workshop"
# 3. Look for version specifications or requirements
rg "ovos-workshop[>=<]"
Length of output: 1165
weather_helpers/intent.py (2)
20-20: LGTM: Import change aligns with PR objective
The replacement of lingua_franca.parse.normalize with ovos_utterance_normalizer.UtteranceNormalizerPlugin is consistent with the goal of dropping the lingua-franca dependency.
42-43: Consider adding error handling for normalizer initialization
While the normalization logic is correct, it could benefit from proper error handling:
- The normalizer initialization might fail if the language is not supported
- The weather_config.lang parameter needs to be properly set
Let's verify the language configuration handling:
Consider adding error handling:
- normalizer = UtteranceNormalizerPlugin.get_normalizer(lang=weather_config.lang)
- self.utterance = normalizer.normalize(message.data["utterance"])
+ try:
+ normalizer = UtteranceNormalizerPlugin.get_normalizer(lang=weather_config.lang)
+ self.utterance = normalizer.normalize(message.data["utterance"])
+ except Exception as e:
+ # Fallback to raw utterance if normalization fails
+ self.utterance = message.data["utterance"]
+ LOG.warning(f"Utterance normalization failed: {str(e)}")weather_helpers/util.py (2)
21-21: LGTM: Clean transition from lingua-franca to ovos_date_parser
The import change successfully replaces lingua-franca with the new ovos_date_parser library while maintaining the required functionality.
68-68: Verify consistent parameter naming across codebase
The parameter name changes from anchor_date to anchorDate and language to lang are correct for the new ovos_date_parser library.
Let's verify that all calls to extract_datetime use the correct parameter names:
✅ Verification successful
Parameter naming is consistent throughout the codebase
The verification shows that:
- There's only one instance of
extract_datetimeusage inweather_helpers/util.py - The function is imported from
ovos_date_parser - The parameter names
anchorDateandlangare correctly used in the only call to this function
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining calls to extract_datetime with old parameter names
rg -l "extract_datetime.*anchor_date|extract_datetime.*language" --type py
# Search for all current extract_datetime usage patterns
rg -A 2 "extract_datetime\(" --type py
Length of output: 362
Script:
#!/bin/bash
# Let's check the actual implementation of extract_datetime to verify the parameter names
ast-grep --pattern 'def extract_datetime($$$)'
# Also check for any imports of extract_datetime to understand its source
rg "from.*import.*extract_datetime|import.*extract_datetime" --type py
Length of output: 203
weather_helpers/dialog.py (3)
42-42: LGTM: Appropriate replacement for lingua-franca dependency.
The join_word_list from ovos_workshop.skills.ovos is a suitable replacement for the removed join_list from lingua_franca.format.
225-225: LGTM: Method signature improvements.
The changes to method signatures are appropriate:
- Formatting fix in
HourlyDialog.build_temperature_dialog - Added default value in
DailyDialog.build_temperature_dialogimproves backward compatibility
Also applies to: 309-309
430-430: LGTM: Improved internationalization support.
The updated join_word_list usage with language-specific parameters (connector, sep, lang) enhances internationalization support.
__init__.py (3)
26-27: LGTM: Import changes align with PR objectives.
The replacement of lingua_franca imports with their ovos counterparts is correct and consistent with the PR's goal.
850-850: LGTM: Language parameter correctly added to function calls.
The addition of the lang=self.lang parameter to nice_weekday and nice_time calls ensures proper language support is maintained after switching to the OVOS libraries.
Also applies to: 1052-1052
Line range hint 1-1150: Verify complete removal of lingua-franca.
Let's ensure all instances of lingua-franca have been replaced throughout the codebase.
✅ Verification successful
Based on the shell script results, there are no remaining references to lingua-franca in the codebase, confirming that the migration to ovos-date-parser and ovos-number-parser is complete.
Migration to OVOS parsers is complete and verified
The verification confirms that all lingua-franca imports and references have been successfully replaced with the OVOS equivalents. No further changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining lingua-franca imports or references
# Search for any remaining lingua-franca imports
echo "Checking for remaining lingua-franca imports..."
rg "from lingua_franca" || echo "No lingua-franca imports found"
rg "import lingua_franca" || echo "No lingua-franca imports found"
# Search for any remaining lingua-franca string references
echo "Checking for remaining lingua-franca string references..."
rg "lingua_franca" || echo "No lingua-franca references found"
Length of output: 521
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores