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

refactor:move_to_extras #315

Merged
merged 1 commit into from
Dec 4, 2024
Merged

refactor:move_to_extras #315

Show file tree
Hide file tree
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
4 changes: 2 additions & 2 deletions ovos_utils/geolocation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@

import requests
from requests.exceptions import RequestException, Timeout
from timezonefinder import TimezoneFinder

from ovos_utils import timed_lru_cache
from ovos_utils.lang import standardize_lang_tag
from ovos_utils.log import LOG
from ovos_utils.network_utils import get_external_ip, is_valid_ip


_tz_finder: TimezoneFinder = None
_tz_finder = None


def get_timezone(lat: float, lon: float) -> Dict[str, str]:
Expand All @@ -30,6 +29,7 @@ def get_timezone(lat: float, lon: float) -> Dict[str, str]:
"""
global _tz_finder
if _tz_finder is None:
from timezonefinder import TimezoneFinder
# lazy loaded, resource intensive so we only want to do it once
_tz_finder = TimezoneFinder()
Comment on lines 31 to 34
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling for missing optional dependency

The function should provide clear feedback when timezonefinder is not installed.

     if _tz_finder is None:
-        from timezonefinder import TimezoneFinder
-        # lazy loaded, resource intensive so we only want to do it once
-        _tz_finder = TimezoneFinder()
+        try:
+            from timezonefinder import TimezoneFinder
+            # lazy loaded, resource intensive so we only want to do it once
+            _tz_finder = TimezoneFinder()
+        except ImportError:
+            LOG.error("timezonefinder not installed. Please install 'ovos-utils[location]' to use timezone functionality")
+            raise RuntimeError("timezonefinder package is required for timezone lookup")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if _tz_finder is None:
from timezonefinder import TimezoneFinder
# lazy loaded, resource intensive so we only want to do it once
_tz_finder = TimezoneFinder()
if _tz_finder is None:
try:
from timezonefinder import TimezoneFinder
# lazy loaded, resource intensive so we only want to do it once
_tz_finder = TimezoneFinder()
except ImportError:
LOG.error("timezonefinder not installed. Please install 'ovos-utils[location]' to use timezone functionality")
raise RuntimeError("timezonefinder package is required for timezone lookup")

try:
Expand Down
8 changes: 1 addition & 7 deletions ovos_utils/gui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ def is_gui_running(applications: List[str] = _default_gui_apps) -> bool:
Return true if a GUI application is running
@param applications: list of applications to check for
"""
deprecated = any((is_process_running(app) for app in applications
if app.startswith("mycroft-")))
if deprecated:
LOG.warning("you are running a deprecated mycroft-gui version, "
"please move to a OVOS maintained version")
return True
return deprecated or any((is_process_running(app) for app in applications))
return any((is_process_running(app) for app in applications))


def is_gui_connected(bus=None) -> bool:
Expand Down
4 changes: 2 additions & 2 deletions ovos_utils/lang/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
from os.path import isdir, join
from typing import Optional

from langcodes import tag_distance, standardize_tag as std

from ovos_utils.file_utils import resolve_resource_file


def standardize_lang_tag(lang_code: str, macro=True) -> str:
"""https://langcodes-hickford.readthedocs.io/en/sphinx/index.html"""
try:
from langcodes import standardize_tag as std
return str(std(lang_code, macro=macro))
except:
if macro:
Expand All @@ -28,6 +27,7 @@ def get_language_dir(base_path: str, lang: str ="en-US") -> Optional[str]:
for f in listdir(base_path):
if isdir(f"{base_path}/{f}"):
try:
from langcodes import tag_distance
score = tag_distance(lang, f)
except: # not a valid language code
continue
Expand Down
2 changes: 1 addition & 1 deletion ovos_utils/oauth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import requests
from json_database import JsonStorageXDG
from oauthlib.oauth2 import WebApplicationClient
from ovos_config.locations import get_xdg_cache_save_path

from ovos_utils.log import LOG
Expand Down Expand Up @@ -108,6 +107,7 @@ def refresh_oauth_token(token_id):
client_secret = app_data["client_secret"]

# Perform refresh
from oauthlib.oauth2 import WebApplicationClient
client = WebApplicationClient(client_id, refresh_token=refresh_token)
uri, headers, body = client.prepare_refresh_token_request(token_endpoint)
refresh_result = requests.post(uri, headers=headers, data=body,
Comment on lines +110 to 113
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for missing optional dependency

The function should gracefully handle missing oauthlib dependency and provide clear feedback to users.

-    from oauthlib.oauth2 import WebApplicationClient
-    client = WebApplicationClient(client_id, refresh_token=refresh_token)
+    try:
+        from oauthlib.oauth2 import WebApplicationClient
+        client = WebApplicationClient(client_id, refresh_token=refresh_token)
+    except ImportError:
+        LOG.error("oauthlib not installed. Please install 'ovos-utils[oauth]' to use OAuth functionality")
+        return None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from oauthlib.oauth2 import WebApplicationClient
client = WebApplicationClient(client_id, refresh_token=refresh_token)
uri, headers, body = client.prepare_refresh_token_request(token_endpoint)
refresh_result = requests.post(uri, headers=headers, data=body,
try:
from oauthlib.oauth2 import WebApplicationClient
client = WebApplicationClient(client_id, refresh_token=refresh_token)
except ImportError:
LOG.error("oauthlib not installed. Please install 'ovos-utils[oauth]' to use OAuth functionality")
return None
uri, headers, body = client.prepare_refresh_token_request(token_endpoint)
refresh_result = requests.post(uri, headers=headers, data=body,

Expand Down
3 changes: 3 additions & 0 deletions requirements/extras.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ ovos-plugin-manager>=0.0.25,<1.0.0
ovos-config>=0.0.12,<2.0.0
ovos-workshop>=0.0.13,<3.0.0
ovos_bus_client>=0.0.8,<2.0.0
langcodes
timezonefinder
oauthlib~=3.2
5 changes: 1 addition & 4 deletions requirements/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,4 @@ pyee>=8.0.0
combo-lock~=0.2
rich-click~=1.7
rich~=13.7
orjson
langcodes
timezonefinder
oauthlib~=3.2
orjson
Loading