feat: list join util #269
feat: list join util #269
Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA series of new JSON files named Changes
Possibly related PRs
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 #269 +/- ##
==========================================
- Coverage 53.41% 51.36% -2.06%
==========================================
Files 37 36 -1
Lines 4362 4213 -149
==========================================
- Hits 2330 2164 -166
- Misses 2032 2049 +17 ☔ 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: 1
🧹 Outside diff range and nitpick comments (3)
ovos_workshop/skills/common_query_skill.py (2)
92-93: LGTM: Consistent language handling in property accessors.The property accessors correctly handle the primary language code extraction. Consider extracting the language code splitting into a private method to avoid code duplication.
+ def _get_primary_lang(self) -> str: + return self.lang.split("-")[0] @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_primary_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_primary_lang()] = valAlso applies to: 98-99
183-183: LGTM: Consistent language handling in remove_noise method.The language parameter handling is correctly implemented. This change would also benefit from using the suggested
_get_primary_langmethod.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_primary_lang()ovos_workshop/skills/ovos.py (1)
2546-2578: Improve type hints and input validationThe function implementation is good but could use some improvements:
- Add type hint for
sepparameter- Add validation for
connectorparameter- Move connector mapping to module level
+ # At module level + SUPPORTED_CONNECTORS = { + "and": _get_connector_word, + "or": _get_connector_word + } - def join_word_list(items: List[str], connector: str, sep: str, lang:str) -> str: + def join_word_list(items: List[str], connector: str, sep: Optional[str], lang: str) -> str: """Join a list into a phrase using the given connector word Examples: join_word_list([1,2,3], "or") -> "1, 2 or 3" join_word_list([1,2,3], "and") -> "1, 2 and 3" join_word_list([1,2,3], "and", ";") -> "1; 2 and 3" Args: items (array): items to be joined connector (str): connecting word ("and" or "or") sep (str, optional): separator character, default = "," lang (str): BCP-47 language code Returns: str: the connected list phrase Raises: ValueError: If connector is not "and" or "or" """ + if connector not in SUPPORTED_CONNECTORS: + raise ValueError(f"Connector must be one of: {list(SUPPORTED_CONNECTORS.keys())}") - cons = { - "and": _get_and_word, - "or": _get_or_word - } if not items: return "" if len(items) == 1: return str(items[0]) if not sep: sep = ", " else: sep += " " return (sep.join(str(item) for item in items[:-1]) + - " " + cons[connector](lang) + + " " + SUPPORTED_CONNECTORS[connector](connector, lang) + " " + items[-1])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
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(2 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 context used
🪛 Ruff
ovos_workshop/skills/ovos.py
2523-2523: Undefined name json
(F821)
2542-2542: Undefined name json
(F821)
🔇 Additional comments (3)
ovos_workshop/skills/common_query_skill.py (2)
70-72: LGTM: Improved language code handling.
The extraction of the primary language code using split("-")[0] is a good practice for resource file handling, ensuring compatibility with locale-specific language codes (e.g., "en-US" → "en").
82-83: LGTM: Proper language-specific noise words storage.
The implementation correctly stores noise words in a dictionary using the primary language code as the key, which is a clean and efficient approach.
ovos_workshop/skills/ovos.py (1)
2067-2067: LGTM: Good improvement to localization
The replacement of join_list with join_word_list in ask_selection improves the localization capabilities by using language-specific connectors.
join word lists with "or" or "and"
split from #268
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
CommonQuerySkillclass to ensure correct processing of noise words based on primary language codes.