fix: drop lingua_franca #268
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #268 +/- ##
==========================================
- Coverage 53.41% 51.41% -2.01%
==========================================
Files 37 36 -1
Lines 4362 4209 -153
==========================================
- Hits 2330 2164 -166
- Misses 2032 2045 +13 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
ovos_workshop/skills/common_query_skill.py (3)
70-72: LGTM! Consider extracting the language code splitting logic.The language code handling is correct, but consider extracting the
split("-")[0]logic into a helper method for better reusability and maintainability.+ def _get_base_lang(self) -> str: + """Extract base language code (e.g., 'en' from 'en-US')""" + return self.lang.split("-")[0] def __init__(self, *args, **kwargs): # ... - lang = self.lang.split("-")[0] + lang = self._get_base_lang()
93-93: LGTM! Consider caching the base language code.The implementation is correct, but the language code splitting is repeated in multiple places. Consider using the suggested
_get_base_lang()method here as well.@property def translated_noise_words(self) -> List[str]: - return self._translated_noise_words.get(self.lang.split("-")[0], []) + return self._translated_noise_words.get(self._get_base_lang(), []) @translated_noise_words.setter def translated_noise_words(self, val: List[str]): - self._translated_noise_words[self.lang.split("-")[0]] = val + self._translated_noise_words[self._get_base_lang()] = valAlso applies to: 99-99
Line range hint
183-189: LGTM! Apply the same language code handling improvement here.The noise word removal logic is correct. For consistency, use the suggested
_get_base_lang()method here as well.def remove_noise(self, phrase: str, lang: str = None) -> str: - lang = (lang or self.lang).split("-")[0] + lang = lang.split("-")[0] if lang else self._get_base_lang()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
ovos_workshop/res/text/az/word_connectors.json(1 hunks)ovos_workshop/res/text/ca/word_connectors.json(1 hunks)ovos_workshop/res/text/cs/word_connectors.json(1 hunks)ovos_workshop/res/text/da/word_connectors.json(1 hunks)ovos_workshop/res/text/de/word_connectors.json(1 hunks)ovos_workshop/res/text/en/word_connectors.json(1 hunks)ovos_workshop/res/text/fa/word_connectors.json(1 hunks)ovos_workshop/res/text/pl/word_connectors.json(1 hunks)ovos_workshop/res/text/sl/word_connectors.json(1 hunks)ovos_workshop/res/text/uk/word_connectors.json(1 hunks)ovos_workshop/skills/common_query_skill.py(4 hunks)ovos_workshop/skills/ovos.py(5 hunks)requirements/requirements.txt(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- ovos_workshop/res/text/az/word_connectors.json
- ovos_workshop/res/text/ca/word_connectors.json
- ovos_workshop/res/text/cs/word_connectors.json
- ovos_workshop/res/text/da/word_connectors.json
- ovos_workshop/res/text/de/word_connectors.json
- ovos_workshop/res/text/en/word_connectors.json
- ovos_workshop/res/text/fa/word_connectors.json
- ovos_workshop/res/text/pl/word_connectors.json
- ovos_workshop/res/text/sl/word_connectors.json
- ovos_workshop/res/text/uk/word_connectors.json
🔇 Additional comments (8)
requirements/requirements.txt (2)
5-6: LGTM! Version constraints follow best practices.
The version constraints >=0.0.1,<1.0.0 follow semantic versioning best practices, allowing patches and minor updates while preventing major breaking changes.
5-6: Verify package stability and feature completeness.
The new packages are at very early versions (0.0.1). Let's verify their stability and ensure they fully replace the removed ovos-lingua-franca functionality.
Consider adding comments in requirements.txt to document that these packages replace lingua_franca functionality:
+# Replacements for ovos-lingua-franca
ovos-solver-yes-no-plugin>=0.0.1,<1.0.0
ovos-number-parser>=0.0.1,<1.0.0✅ Verification successful
Package versions and functionality are stable and complete
The verification shows:
- ovos-solver-yes-no-plugin is at v0.1.0a1 (not 0.0.1) with comprehensive yes/no parsing capabilities
- ovos-number-parser v0.0.1 provides full number parsing functionality across multiple languages
- No remaining lingua_franca imports found in the codebase, indicating complete migration
The packages appear to be stable replacements for the removed lingua_franca functionality:
- Yes/No solver handles affirmative/negative response parsing with extensive test coverage
- Number parser supports number extraction, pronunciation, ordinals and fractions across 19 languages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check package information and recent activity
# Check PyPI metadata for both packages
echo "Checking ovos-solver-yes-no-plugin PyPI info..."
curl -s "https://pypi.org/pypi/ovos-solver-yes-no-plugin/json" | jq -r '.info | {version: .version, author: .author, description: .description}'
echo "Checking ovos-number-parser PyPI info..."
curl -s "https://pypi.org/pypi/ovos-number-parser/json" | jq -r '.info | {version: .version, author: .author, description: .description}'
# Search for any remaining lingua_franca imports that might need updating
echo "Checking for remaining lingua_franca imports..."
rg -l "from lingua_franca" || echo "No lingua_franca imports found"
rg -l "import lingua_franca" || echo "No lingua_franca imports found"
Length of output: 10507
ovos_workshop/skills/common_query_skill.py (2)
83-84: LGTM! Clean implementation of language-specific noise words storage.
The dictionary-based storage using base language codes is a good approach for managing multilingual noise words.
Line range hint 70-189: Verify language resource files.
Let's ensure all required language resource files are present in the repository.
✅ Verification successful
Language resource files are properly structured and available
The verification shows that noise word files are present for multiple languages (cs, de, en, ru) in the expected location ovos_workshop/res/text/{lang}/noise_words.list. Each file contains appropriate noise words for its respective language, properly formatted with one word per line.
The implementation correctly:
- Handles language codes by splitting on "-" (e.g., "en-US" → "en")
- Provides default resource paths that match the actual file structure
- Contains valid content in the noise word files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for language resource files in the repository
# Test: List all noise_words.list files
echo "Checking noise words files..."
fd "noise_words.list" --type f
# Test: Check content format of noise words files
echo -e "\nVerifying noise words file format..."
fd "noise_words.list" --type f --exec cat {} \; | grep -v '^[[:space:]]*$' || true
Length of output: 1183
ovos_workshop/skills/ovos.py (4)
2-2: Necessary import of json module
The json module is correctly imported to handle JSON operations in the newly added helper functions.
19-20: Update imports to new dependencies
The import statements are updated to use ovos_number_parser and ovos_yes_no_solver, replacing lingua_franca functions. This aligns with the shift to the new dependencies and ensures compatibility with the updated libraries.
2013-2013: Initialize YesNoSolver in ask_yesno method
The ask_yesno method now utilizes the YesNoSolver class to improve the accuracy of yes/no response handling, enhancing language support and flexibility.
2059-2061: Utilize join_word_list for localized option presentation
The ask_selection method now uses the join_word_list function to present options in a localized and readable format, improving internationalization and user experience.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe changes in this pull request involve significant updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (2)ovos_workshop/skills/ovos.py (2)
The replacement of lingua_franca with ovos_number_parser and ovos_yes_no_solver is appropriate and maintains the required functionality.
The implementation correctly replaces lingua_franca's yes_or_no with YesNoSolver while maintaining the same functionality and return value contract. 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 (
|
Summary by CodeRabbit
New Features
YesNoSolver.pronounce_numbermethod.Bug Fixes
Chores
ovos-lingua-francaand addingovos-solver-yes-no-pluginandovos-number-parser.