fix:lingua_franca #257
fix:lingua_franca #257
Conversation
lingua franca requires lowercased lang codes, which causes ask_yesno to fail adds better dialect support
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #257 +/- ##
==========================================
- Coverage 53.41% 51.49% -1.93%
==========================================
Files 37 36 -1
Lines 4362 4193 -169
==========================================
- Hits 2330 2159 -171
- Misses 2032 2034 +2 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| # TODO - lingua_franca does not support standardized lang tags | ||
| # deprecate it, use a plugin here https://github.com/TigreGotico/ovos-solver-YesNo-plugin | ||
| lf_langs = ("az-az", "ca-es", "cs-cz", "da-dk", "de-de", | ||
| "en-us", "es-es", "fr-fr", | ||
| "hu-hu", "it-it", "nl-nl", "pl-pl", | ||
| "fa-ir", "pt-pt", "ru-ru", "sl-si", | ||
| "sv-se", "tr-tr", "eu-eu", "uk-ua") | ||
| lang, score = closest_match(self.lang, lf_langs) | ||
| if score > 10: | ||
| lang = self.lang # let it raise a value Error in next line | ||
| answer = yes_or_no(resp, lang=lang) if resp else resp |
There was a problem hiding this comment.
💡 Codebase verification
Issue Found: yes_or_no from lingua_franca is Still in Use
The yes_or_no function from lingua_franca.parse is still being imported and utilized in ovos_workshop/skills/ovos.py. This indicates that the deprecation of lingua_franca has not been fully implemented as intended.
ovos_workshop/skills/ovos.py: from lingua_franca.parse import yes_or_no, extract_numberovos_workshop/skills/ovos.py: answer = yes_or_no(resp, lang=lang) if resp else resp
🔗 Analysis chain
Approve changes addressing language code issue
The modifications to handle language codes and use closest_match effectively address the issue mentioned in the PR objectives. This solution provides better support for various language codes and dialects.
Consider adding a comment explaining the closest_match logic and why a score of 10 was chosen as the threshold. This will help future maintainers understand the reasoning behind this implementation.
lang, score = closest_match(self.lang, lf_langs)
+ # Use a threshold of 10 to determine if the matched language is close enough
+ # Scores: 0-3 (minor difference), 4-10 (significant but acceptable difference)
if score > 10:
lang = self.lang # let it raise a ValueError in the next lineThe TODO comment suggests deprecating lingua_franca in favor of a plugin-based approach. Would you like me to create a GitHub issue to track this task for future implementation?
Let's verify the impact of these changes on the rest of the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of yes_or_no function from lingua_franca
rg "from lingua_franca.parse import yes_or_no" --type python
rg "yes_or_no\(" --type python
Length of output: 157
Script:
#!/bin/bash
# Retry searching for other usages of yes_or_no function using the correct type identifier
rg "from lingua_franca.parse import yes_or_no" -t py
rg "yes_or_no\(" -t py
Length of output: 257
There was a problem hiding this comment.
open an issue to track this
There was a problem hiding this comment.
@JarbasAl, I've opened issue #259 to track this task.
🔗 Analysis chain
@JarbasAl, I've opened a GitHub issue to track this task.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Create a new GitHub issue to track the deprecation of 'lingua_franca' in favor of a plugin-based approach
gh issue create --title "Deprecate 'lingua_franca' in favor of plugin-based approach" \
--body "As discussed in [PR #257](https://github.com/OpenVoiceOS/OVOS-workshop/pull/257) and [this comment](https://github.com/OpenVoiceOS/OVOS-workshop/pull/257#discussion_r1805688426) by @JarbasAl, we need to deprecate the 'lingua_franca' library in favor of a plugin-based approach. This issue is to track this task for future implementation.
" \
--assignee JarbasAl
Length of output: 524
lingua franca requires lowercased lang codes, which causes ask_yesno to fail
adds better dialect support
closes #256
Summary by CodeRabbit
New Features
ask_yesnomethod, improving localization support.Bug Fixes
Documentation