-
Notifications
You must be signed in to change notification settings - Fork 13
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
fix:langcodes #241
fix:langcodes #241
Conversation
the lang code standard assumes region is upper case, however mycroft forced a lower case lang code which is now causing issues if the standard is used besides handling this, dialect support is now improved by using langcodes library distance function, ensuring the best dialect is selected
WalkthroughThe changes in this pull request involve modifications to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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 (
|
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- ovos_workshop/app.py (2 hunks)
- ovos_workshop/resource_files.py (2 hunks)
🧰 Additional context used
🔇 Additional comments (3)
ovos_workshop/app.py (2)
5-5
: Import ofstandardize_lang_tag
is appropriate.The import statement correctly brings in
standardize_lang_tag
, which is used to standardize language tags in the code below.
95-95
: Clarify the comment regarding sorted directories.The comment correctly notes that the directories returned by
locate_lang_directories
are already sorted, with the first being the best match. This provides clear guidance on the selection logic.ovos_workshop/resource_files.py (1)
87-96
: Improved language code matching usingtag_distance
The updated logic in
locate_lang_directories
enhances language directory selection by evaluating the distance between language codes usingtag_distance
. This allows for better matching of language variants and dialects. The implementation appears correct and should improve the functionality as intended.
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
🧹 Outside diff range and nitpick comments (1)
requirements/requirements.txt (1)
7-7
: Consider adding a version constraint for langcodesThe addition of
langcodes
is appropriate and aligns with the PR objectives for improving dialect support. However, it's recommended to specify a version constraint to ensure compatibility and reproducibility.Consider updating the line to include a version constraint, for example:
-langcodes +langcodes>=1.4.1,<2.0.0Replace
1.4.1
with the minimum version that includes the features you need. This will help prevent potential issues if breaking changes are introduced in future versions oflangcodes
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- requirements/requirements.txt (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
requirements/requirements.txt (1)
1-1
: LGTM: Version update for ovos-utilsThe update to
ovos-utils>=0.2.1,<1.0.0
is appropriate. It increases the minimum required version, which likely includes necessary features or bug fixes, while maintaining the same upper bound for compatibility.To ensure compatibility, please verify that the project works correctly with the new minimum version of
ovos-utils
. You can use the following command to check for any potential issues:
no longer applies when latest ovos-utils is installed
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #241 +/- ##
==========================================
- Coverage 53.41% 49.16% -4.26%
==========================================
Files 37 37
Lines 4362 4365 +3
==========================================
- Hits 2330 2146 -184
- Misses 2032 2219 +187 ☔ View full report in Codecov by Sentry. |
closes #243 |
the lang code standard assumes region is upper case, however mycroft forced a lower case lang code which is now causing issues if the standard is used
besides handling this, dialect support is now improved by using langcodes library distance function, ensuring the best dialect is selected
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
ovos-utils
package and added a new dependency,langcodes
.Tests