Skip to content

Commit acbd438

Browse files
JarbasAlclaude
andauthored
fix: update locale lookups and tests after BCP-47 folder rename (#395)
* fix: update locale lookups and tests after BCP-47 folder rename The recent rename of locale folders from short codes (en-us, it, es) to canonical BCP-47 tags (en-US, it-IT, es-ES) broke two things: 1. _get_word() stripped the region tag and looked for locale/it/ which no longer exists. Fix: try the full tag first, then the short code, then scan for any folder matching the language prefix. 2. test_base.py built expected file paths using the old lowercase folder names (en-us, uk-ua). Fix: update to en-US / uk-UA to match the renamed directories. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: use get_language_dir() from ovos-utils for all locale lookups Replace ad-hoc lang string splitting and manual locale path construction with ovos_utils.lang.get_language_dir(), which uses langcodes.tag_distance() to find the best matching folder regardless of casing or regional variants. - _get_dialog(): no longer strips region tag before building path - _get_word(): replaces multi-step fallback scan with single get_language_dir() call - CommonQuerySkill.__init__(): uses get_language_dir() instead of lang.split("-")[0] - CommonQuerySkill translated_noise_words property/setter: use full lang tag as dict key Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: add locale lookup and language resource tests Validates the refactored get_language_dir()-based lookups: - TestGetWord: canonical BCP-47, short code ('it', 'es', 'en'), lowercase tags ('en-us'), missing lang fallback, and a structural check that every locale folder's word_connectors.json has 'and'/'or' - TestGetDialog: resolves bundled .dialog files by canonical, lowercase, and short-code lang tags; verifies context rendering; verifies fallback for missing file and unknown lang - TestJoinWordList: end-to-end for English, Italian (euphony e→ed, o→od), Spanish (euphony y→e, o→u), German, French; also single-item and empty-list edge cases; short-code equivalence checks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: route all workshop locale lookups through CoreResources _get_dialog() and _get_word() were still building paths manually even after the get_language_dir() step. CoreResources / SkillResources in resource_files.py already handle the full lookup chain (locale/, dialog/, vocab/ subdirectories, user overrides, tag distance matching) — there is no reason to duplicate that logic. - _get_dialog(): delegate to CoreResources(lang).load_dialog_file() - _get_word(): delegate to CoreResources(lang).load_json_file() - CommonQuerySkill.__init__: delegate to CoreResources(lang).load_list_file() - Remove manual path construction, os.listdir scans, resolve_resource_file calls and the get_language_dir import that were introduced in earlier steps — resource_files.py is the canonical place for this Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use self.resources inside skill class instead of calling _get_dialog ResourceFile._locate() already falls back to workshop_directory (the matching lang dir in ovos_workshop/locale/) so self.resources will find both skill-local AND workshop-bundled files. There is no reason for code inside the skill class to call the module-level _get_dialog() helper — that function is only needed outside a skill instance context (e.g. in join_word_list which has no self). _get_dialog / _get_word / CoreResources are still the right tool for module-level utilities. Inside the skill, self.resources is canonical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove redundant CoreResources fallback in voc_list self.resources already searches workshop_directory (ovos_workshop/locale/{lang}/) as a fallback in ResourceFile._locate(), so the explicit CoreResources(lang) fallback was redundant. CoreResources now only appears in module-level helpers (_get_dialog, _get_word) that have no skill instance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: remove unused method * chore: remove unused method * fix: use self.resources in CommonQuerySkill instead of CoreResources self.resources already searches workshop_directory as a fallback, so CoreResources was redundant here too. CoreResources is now only used in module-level free functions (_get_dialog, _get_word) that have no self. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: replace _get_dialog calls in game_skill with speak_dialog _get_dialog no longer exists as a public symbol; game_skill.py was importing it. speak_dialog(key) uses self.resources (which already falls back to workshop_directory) and is the correct API for skills. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: align remove_noise() lang key with _translated_noise_words cache Cache is written with the full BCP-47 tag (e.g. "en-US") but remove_noise() stripped to the base subtag ("en") before lookup, so the dict lookup always missed. Use the full tag consistently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: remove deprecation flagged for 0.1.0 --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 99fccf8 commit acbd438

5 files changed

Lines changed: 185 additions & 86 deletions

File tree

ovos_workshop/skills/common_query_skill.py

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@
1212

1313
from abc import abstractmethod
1414
from enum import IntEnum
15-
from os.path import dirname
1615
from typing import List, Optional, Tuple
1716

1817
from ovos_bus_client import Message
19-
from ovos_utils.file_utils import resolve_resource_file
2018
from ovos_utils.log import LOG, log_deprecation
2119
import warnings
2220
from ovos_workshop.skills.ovos import OVOSSkill
@@ -73,36 +71,11 @@ def __init__(self, *args, **kwargs):
7371
}
7472
super().__init__(*args, **kwargs)
7573

76-
lang = self.lang.split("-")[0]
77-
noise_words_filepath = f"text/{lang}/noise_words.list"
78-
default_res = f"{dirname(dirname(__file__))}/locale/{lang}" \
79-
f"/noise_words.list"
80-
noise_words_filename = \
81-
resolve_resource_file(noise_words_filepath,
82-
config=self.config_core) or \
83-
resolve_resource_file(default_res, config=self.config_core)
84-
74+
lang = self.lang
75+
noise_words = self.resources.load_list_file("noise_words")
8576
self._translated_noise_words = {}
86-
if noise_words_filename:
87-
with open(noise_words_filename) as f:
88-
translated_noise_words = f.read().strip()
89-
self._translated_noise_words[lang] = \
90-
translated_noise_words.split()
91-
92-
@property
93-
def translated_noise_words(self) -> List[str]:
94-
"""
95-
Get a list of "noise" words in the current language
96-
"""
97-
log_deprecation("self.translated_noise_words will become a "
98-
"private variable", "0.1.0")
99-
return self._translated_noise_words.get(self.lang.split("-")[0], [])
100-
101-
@translated_noise_words.setter
102-
def translated_noise_words(self, val: List[str]):
103-
log_deprecation("self.translated_noise_words will become a "
104-
"private variable", "0.1.0")
105-
self._translated_noise_words[self.lang.split("-")[0]] = val
77+
if noise_words:
78+
self._translated_noise_words[lang] = noise_words
10679

10780
def bind(self, bus):
10881
"""Overrides the default bind method of MycroftSkill.
@@ -186,7 +159,7 @@ def remove_noise(self, phrase: str, lang: str = None) -> str:
186159
@param lang: language of `phrase`, else defaults to `self.lang`
187160
@return: cleaned `phrase` with extra words removed
188161
"""
189-
lang = (lang or self.lang).split("-")[0]
162+
lang = lang or self.lang
190163
phrase = ' ' + phrase + ' '
191164
for word in self._translated_noise_words.get(lang, []):
192165
mtch = ' ' + word + ' '

ovos_workshop/skills/game_skill.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
from ovos_workshop.decorators import ocp_featured_media, ocp_search
1010
from ovos_workshop.skills.common_play import OVOSCommonPlaybackSkill
11-
from ovos_workshop.skills.ovos import _get_dialog
1211

1312

1413
class OVOSGameSkill(OVOSCommonPlaybackSkill):
@@ -152,31 +151,27 @@ class ConversationalGameSkill(OVOSGameSkill):
152151

153152
def on_save_game(self):
154153
"""skills can override method to implement functioonality"""
155-
speech = _get_dialog("cant_save_game", self.lang)
156-
self.speak(speech)
154+
self.speak_dialog("cant_save_game")
157155

158156
def on_load_game(self):
159157
"""skills can override method to implement functioonality"""
160-
speech = _get_dialog("cant_load_game", self.lang)
161-
self.speak(speech)
158+
self.speak_dialog("cant_load_game")
162159

163160
def on_pause_game(self):
164161
"""called by ocp_pipeline on 'pause' if game is being played"""
165162
self._paused.set()
166163
self.acknowledge()
167164
# individual skills can change default value if desired
168165
if self.settings.get("pause_dialog", False):
169-
speech = _get_dialog("game_pause", self.lang)
170-
self.speak(speech)
166+
self.speak_dialog("game_pause")
171167

172168
def on_resume_game(self):
173169
"""called by ocp_pipeline on 'resume/unpause' if game is being played and paused"""
174170
self._paused.clear()
175171
self.acknowledge()
176172
# individual skills can change default value if desired
177173
if self.settings.get("pause_dialog", False):
178-
speech = _get_dialog("game_unpause", self.lang)
179-
self.speak(speech)
174+
self.speak_dialog("game_unpause")
180175

181176
@abc.abstractmethod
182177
def on_play_game(self):

ovos_workshop/skills/ovos.py

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1476,7 +1476,8 @@ def _on_event_error(self, error: str, message: Message, handler_info: str,
14761476
# Convert "MyFancySkill" to "My Fancy Skill" for speaking
14771477
handler_name = camel_case_split(self.name)
14781478
msg_data = {'skill': handler_name}
1479-
speech = _get_dialog('skill.error', self.lang, msg_data)
1479+
lines = self.resources.load_dialog_file('skill.error', data=msg_data)
1480+
speech = lines[0] if lines else 'skill.error'
14801481
if speak_errors:
14811482
self.speak(speech)
14821483
self.log.exception(error)
@@ -2028,8 +2029,7 @@ def voc_list(self, voc_filename: str,
20282029
cache_key = lang + voc_filename
20292030

20302031
if cache_key not in self._voc_cache:
2031-
vocab = self.resources.load_vocabulary_file(voc_filename) or \
2032-
CoreResources(lang).load_vocabulary_file(voc_filename)
2032+
vocab = self.resources.load_vocabulary_file(voc_filename)
20332033
if vocab:
20342034
self._voc_cache[cache_key] = list(chain(*vocab))
20352035

@@ -2413,34 +2413,6 @@ def __init__(self, skill: OVOSSkill):
24132413
ui_directories=ui_directories)
24142414

24152415

2416-
def _get_dialog(phrase: str, lang: str, context: Optional[dict] = None) -> str:
2417-
"""
2418-
Looks up a resource file for the given phrase in the specified language.
2419-
2420-
Meant only for resources bundled with ovos-workshop and shared across skills
2421-
2422-
Args:
2423-
phrase (str): resource phrase to retrieve/translate
2424-
lang (str): the language to use
2425-
context (dict): values to be inserted into the string
2426-
2427-
Returns:
2428-
str: a randomized and/or translated version of the phrase
2429-
"""
2430-
lang = standardize_lang_tag(lang).split('-')[0]
2431-
filename = f"{dirname(dirname(__file__))}/locale/{lang}/{phrase}.dialog"
2432-
2433-
if not isfile(filename):
2434-
LOG.debug(f'Resource file not found: {filename}')
2435-
return phrase
2436-
2437-
stache = MustacheDialogRenderer()
2438-
stache.load_template_file('template', filename)
2439-
if not context:
2440-
context = {}
2441-
return stache.render('template', context)
2442-
2443-
24442416
def _get_word(lang, connector):
24452417
""" Helper to get word translations
24462418
@@ -2451,15 +2423,11 @@ def _get_word(lang, connector):
24512423
Returns:
24522424
str: translated version of resource name
24532425
"""
2454-
lang = standardize_lang_tag(lang).split("-")[0]
2455-
res_file = f"{dirname(dirname(__file__))}/locale/{lang}" \
2456-
f"/word_connectors.json"
2457-
if not os.path.isfile(res_file):
2458-
LOG.warning(f"untranslated file: {res_file}")
2459-
return ", "
2460-
with open(res_file) as f:
2461-
w = json.load(f)[connector]
2462-
return w
2426+
data = CoreResources(lang).load_json_file("word_connectors")
2427+
if connector in data:
2428+
return data[connector]
2429+
LOG.warning(f"untranslated word connector '{connector}' for lang: {lang}")
2430+
return ", "
24632431

24642432

24652433
def join_word_list(items: List[str], connector: str, sep: str, lang: str) -> str:

test/unittests/skills/test_base.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,8 @@ def test_register_intent_file(self):
292292
skill._lang_resources = dict()
293293
skill.intent_service = Mock()
294294
skill.res_dir = join(dirname(__file__), "test_locale")
295-
en_intent_file = join(skill.res_dir, "locale", "en-us", "time.intent")
296-
uk_intent_file = join(skill.res_dir, "locale", "uk-ua", "time.intent")
295+
en_intent_file = join(skill.res_dir, "locale", "en-US", "time.intent")
296+
uk_intent_file = join(skill.res_dir, "locale", "uk-UA", "time.intent")
297297

298298
# No secondary languages
299299
skill.config_core["lang"] = "en-US"
@@ -318,8 +318,8 @@ def test_register_entity_file(self):
318318
skill._lang_resources = dict()
319319
skill.intent_service = Mock()
320320
skill.res_dir = join(dirname(__file__), "test_locale")
321-
en_file = join(skill.res_dir, "locale", "en-us", "dow.entity")
322-
uk_file = join(skill.res_dir, "locale", "uk-ua", "dow.entity")
321+
en_file = join(skill.res_dir, "locale", "en-US", "dow.entity")
322+
uk_file = join(skill.res_dir, "locale", "uk-UA", "dow.entity")
323323

324324
# No secondary languages
325325
skill.config_core["lang"] = "en-US"
@@ -331,7 +331,7 @@ def test_register_entity_file(self):
331331

332332
# With secondary language
333333
skill.intent_service.register_padatious_entity.reset_mock()
334-
skill.config_core["secondary_langs"] = ["en-US", "uk-ua"]
334+
skill.config_core["secondary_langs"] = ["en-US", "uk-UA"]
335335
skill.register_entity_file("dow")
336336
self.assertEqual(
337337
skill.intent_service.register_padatious_entity.call_count, 2)
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
"""
2+
Tests for locale directory lookup and language resource resolution.
3+
4+
Covers:
5+
- _get_word() resolves word_connectors.json via get_language_dir()
6+
- join_word_list() produces correct output for all supported language
7+
variants, including case-insensitive and short-code inputs
8+
- All locale folders present in ovos_workshop/locale/ have valid
9+
word_connectors.json with "and"/"or" keys
10+
"""
11+
import json
12+
import os
13+
import unittest
14+
from os.path import dirname, join
15+
16+
from ovos_workshop.skills.ovos import _get_word, join_word_list
17+
18+
LOCALE_DIR = join(dirname(dirname(dirname(__file__))),
19+
"ovos_workshop", "locale")
20+
21+
22+
class TestGetWord(unittest.TestCase):
23+
"""_get_word() must resolve connectors for all supported langs."""
24+
25+
def test_canonical_tag(self):
26+
self.assertEqual(_get_word("en-US", "and"), "and")
27+
self.assertEqual(_get_word("en-US", "or"), "or")
28+
29+
def test_short_code_resolves(self):
30+
"""Plain 'en' should match en-US folder."""
31+
self.assertEqual(_get_word("en", "and"), "and")
32+
33+
def test_lowercase_tag_resolves(self):
34+
"""en-us (all-lowercase) must still resolve."""
35+
self.assertEqual(_get_word("en-us", "and"), "and")
36+
37+
def test_italian_canonical(self):
38+
self.assertEqual(_get_word("it-IT", "and"), "e")
39+
self.assertEqual(_get_word("it-IT", "or"), "o")
40+
41+
def test_italian_short_code(self):
42+
"""'it' must resolve to it-IT folder."""
43+
self.assertEqual(_get_word("it", "and"), "e")
44+
self.assertEqual(_get_word("it", "or"), "o")
45+
46+
def test_spanish_canonical(self):
47+
self.assertEqual(_get_word("es-ES", "and"), "y")
48+
self.assertEqual(_get_word("es-ES", "or"), "o")
49+
50+
def test_spanish_short_code(self):
51+
self.assertEqual(_get_word("es", "and"), "y")
52+
self.assertEqual(_get_word("es", "or"), "o")
53+
54+
def test_german(self):
55+
self.assertEqual(_get_word("de-DE", "and"), "und")
56+
57+
def test_french(self):
58+
self.assertEqual(_get_word("fr-FR", "and"), "et")
59+
60+
def test_missing_lang_returns_fallback(self):
61+
"""Unknown language must return ', ' not raise."""
62+
result = _get_word("xx-XX", "and")
63+
self.assertEqual(result, ", ")
64+
65+
def test_all_locale_folders_have_connectors(self):
66+
"""Every locale folder must contain a parseable word_connectors.json
67+
with both 'and' and 'or' keys."""
68+
for folder in os.listdir(LOCALE_DIR):
69+
path = join(LOCALE_DIR, folder, "word_connectors.json")
70+
if not os.path.isfile(path):
71+
continue # not every locale needs connectors
72+
with open(path) as f:
73+
data = json.load(f)
74+
self.assertIn("and", data,
75+
f"{folder}/word_connectors.json missing 'and'")
76+
self.assertIn("or", data,
77+
f"{folder}/word_connectors.json missing 'or'")
78+
79+
80+
class TestJoinWordList(unittest.TestCase):
81+
"""join_word_list() end-to-end for several languages and input shapes."""
82+
83+
# --- English ---
84+
def test_en_two_items_and(self):
85+
self.assertEqual(join_word_list(["a", "b"], "and", ",", "en-US"),
86+
"a and b")
87+
88+
def test_en_three_items_and(self):
89+
self.assertEqual(join_word_list(["a", "b", "c"], "and", ",", "en-US"),
90+
"a, b and c")
91+
92+
def test_en_two_items_or(self):
93+
self.assertEqual(join_word_list(["x", "y"], "or", ",", "en-US"),
94+
"x or y")
95+
96+
def test_en_single_item(self):
97+
self.assertEqual(join_word_list(["only"], "and", ",", "en-US"), "only")
98+
99+
def test_en_empty(self):
100+
self.assertEqual(join_word_list([], "and", ",", "en-US"), "")
101+
102+
# --- Italian (euphony) ---
103+
def test_it_and_basic(self):
104+
self.assertEqual(
105+
join_word_list(["mare", "montagna"], "and", ",", "it-IT"),
106+
"mare e montagna")
107+
108+
def test_it_and_euphonic(self):
109+
"""'e' + vowel 'e' → 'ed'"""
110+
self.assertEqual(
111+
join_word_list(["inverno", "estate"], "and", ",", "it-IT"),
112+
"inverno ed estate")
113+
114+
def test_it_or_euphonic(self):
115+
"""'o' + vowel 'o' → 'od'"""
116+
self.assertEqual(
117+
join_word_list(["mare", "oceano"], "or", ",", "it-IT"),
118+
"mare od oceano")
119+
120+
def test_it_short_code(self):
121+
"""Short code 'it' must produce the same result as 'it-IT'."""
122+
self.assertEqual(
123+
join_word_list(["mare", "montagna"], "and", ",", "it"),
124+
join_word_list(["mare", "montagna"], "and", ",", "it-IT"))
125+
126+
# --- Spanish (euphony) ---
127+
def test_es_and_euphonic(self):
128+
"""'y' before 'i' → 'e'"""
129+
self.assertEqual(
130+
join_word_list(["Juan", "Irene"], "and", ",", "es-ES"),
131+
"Juan e Irene")
132+
133+
def test_es_or_euphonic(self):
134+
"""'o' before 'o' → 'u'"""
135+
self.assertEqual(
136+
join_word_list(["uno", "otro"], "or", ",", "es-ES"),
137+
"uno u otro")
138+
139+
def test_es_and_no_euphony(self):
140+
self.assertEqual(
141+
join_word_list(["tierra", "agua"], "and", ",", "es-ES"),
142+
"tierra y agua")
143+
144+
def test_es_short_code(self):
145+
self.assertEqual(
146+
join_word_list(["tierra", "agua"], "and", ",", "es"),
147+
join_word_list(["tierra", "agua"], "and", ",", "es-ES"))
148+
149+
# --- German ---
150+
def test_de_and(self):
151+
self.assertEqual(
152+
join_word_list(["Hund", "Katze"], "and", ",", "de-DE"),
153+
"Hund und Katze")
154+
155+
# --- French ---
156+
def test_fr_and(self):
157+
self.assertEqual(
158+
join_word_list(["chien", "chat"], "and", ",", "fr-FR"),
159+
"chien et chat")
160+
161+
162+
if __name__ == "__main__":
163+
unittest.main()

0 commit comments

Comments
 (0)