Skip to content
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.

Sign up for GitHub

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:lingua_franca #257

Merged
merged 1 commit into from
Oct 18, 2024
Merged

fix:lingua_franca #257

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion ovos_workshop/skills/ovos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1999,7 +1999,17 @@ def ask_yesno(self, prompt: str,
'no', including a response of None.
"""
resp = self.get_response(dialog=prompt, data=data)
answer = yes_or_no(resp, lang=self.lang) if resp else resp
# 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
Comment on lines +2002 to +2012
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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_number
  • ovos_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 line

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open an issue to track this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

if answer is True:
return "yes"
elif answer is False:
Expand Down
Loading