Merge pull request #2423 from HullSeals/enhancement/1775/replace-asserts

[#1775] Retire Usage of Assert

config/linux.py

@@ -13,7 +13,8 @@ import sys
 from configparser import ConfigParser
 from config import AbstractConfig, appname, logger
 
-assert sys.platform == 'linux'
+if sys.platform != 'linux':
+    raise EnvironmentError("This file is for Linux only.")
 
 
 class LinuxConfig(AbstractConfig):

config/windows.py

@@ -16,7 +16,8 @@ from typing import Literal
 from config import AbstractConfig, applongname, appname, logger
 from win32comext.shell import shell
 
-assert sys.platform == 'win32'
+if sys.platform != 'win32':
+    raise EnvironmentError("This file is for Windows only.")
 
 REG_RESERVED_ALWAYS_ZERO = 0
 

coriolis-update-files.py

@@ -20,12 +20,15 @@ import sys
 import outfitting
 from edmc_data import coriolis_ship_map, ship_name_map
 
-if __name__ == "__main__":
+if __name__ == "__main__":  # noqa: C901
 
     def add(modules, name, attributes) -> None:
         """Add the given module to the modules dict."""
-        assert name not in modules or modules[name] == attributes, f'{name}: {modules.get(name)} != {attributes}'
-        assert name not in modules, name
+        if name in modules and modules[name] != attributes:
+            raise ValueError(f'{name}: {modules.get(name)} != {attributes}')
+        if name in modules:
+            raise ValueError(f'{name} already exists in modules')
+
         modules[name] = attributes
 
     try:
@@ -50,7 +53,8 @@ if __name__ == "__main__":
     # Ship and armour masses
     for m in list(data['Ships'].values()):
         name = coriolis_ship_map.get(m['properties']['name'], str(m['properties']['name']))
-        assert name in reverse_ship_map, name
+        if name not in reverse_ship_map:
+            raise ValueError(f'Unknown ship: {name}')
         ships[name] = {'hullMass': m['properties']['hullMass'],
                        'reserveFuelCapacity': m['properties']['reserveFuelCapacity']}
         for i, bulkhead in enumerate(bulkheads):
@@ -64,7 +68,8 @@ if __name__ == "__main__":
     for cat in list(data['Modules'].values()):
         for grp, mlist in list(cat.items()):
             for m in mlist:
-                assert 'symbol' in m, m
+                if 'symbol' not in m:
+                    raise ValueError(f'No symbol in {m}')
                 key = str(m['symbol'].lower())
                 if grp == 'fsd':
                     modules[key] = {

edshipyard.py

@@ -128,7 +128,7 @@ def export(data, filename=None) -> None:  # noqa: C901, CCR001
                 elif not slot.lower().startswith('planetaryapproachsuite'):
                     logger.debug(f'EDShipyard: Unknown slot {slot}')
 
-        except AssertionError as e:
+        except ValueError as e:
             logger.debug(f'EDShipyard: {e!r}')
             continue  # Silently skip unrecognized modules
 
@@ -161,8 +161,11 @@ def export(data, filename=None) -> None:  # noqa: C901, CCR001
     string += f'---\nCargo : {cargo} T\nFuel  : {fuel} T\n'
 
     # Add mass and range
-    assert data['ship']['name'].lower() in ship_name_map, data['ship']['name']
-    assert ship_name_map[data['ship']['name'].lower()] in ships, ship_name_map[data['ship']['name'].lower()]
+    ship_name = data['ship']['name'].lower()
+    if ship_name not in ship_name_map:
+        raise ValueError(f"Ship name '{data['ship']['name']}' not found in ship_name_map")
+    if ship_name_map[ship_name] not in ships:
+        raise ValueError(f"Mapped ship name '{ship_name_map[ship_name]}' not found in ships")
 
     try:
         mass += ships[ship_name_map[data['ship']['name'].lower()]]['hullMass']

hotkey/linux.py

@@ -5,7 +5,8 @@ import sys
 from EDMCLogging import get_main_logger
 from hotkey import AbstractHotkeyMgr
 
-assert sys.platform == 'linux'
+if sys.platform != 'linux':
+    raise EnvironmentError("This file is for Linux only.")
 
 logger = get_main_logger()
 

hotkey/windows.py

@@ -17,7 +17,8 @@ from config import config
 from EDMCLogging import get_main_logger
 from hotkey import AbstractHotkeyMgr
 
-assert sys.platform == 'win32'
+if sys.platform != 'win32':
+    raise EnvironmentError("This file is for Windows only.")
 
 logger = get_main_logger()
 

killswitch.py

@@ -275,7 +275,8 @@ class KillSwitchSet:
             return True, data
 
         if TYPE_CHECKING:  # pyright, mypy, please -_-
-            assert res.kill is not None
+            if res.kill is None:
+                raise ValueError('Killswitch has rules but no kill data')
 
         try:
             new_data = res.kill.apply_rules(deepcopy(data))

l10n.py

@@ -133,7 +133,8 @@ class Translations:
 
     def contents(self, lang: str, plugin_path: pathlib.Path | None = None) -> dict[str, str]:
         """Load all the translations from a translation file."""
-        assert lang in self.available()
+        if lang not in self.available():
+            raise KeyError(f'Language {lang} not available')
         translations = {}
 
         h = self.file(lang, plugin_path)

monitor.py

@@ -311,7 +311,8 @@ class EDLogs(FileSystemEventHandler):
         if self.observed:
             logger.debug('self.observed: Calling unschedule_all()')
             self.observed = None
-            assert self.observer is not None, 'Observer was none but it is in use?'
+            if self.observer is None:
+                raise RuntimeError("Observer was None but it is in use?")
             self.observer.unschedule_all()
             logger.debug('Done')
 
@@ -416,7 +417,8 @@ class EDLogs(FileSystemEventHandler):
         # Watchdog thread -- there is a way to get this by using self.observer.emitters and checking for an attribute:
         # watch, but that may have unforseen differences in behaviour.
         if self.observed:
-            assert self.observer is not None, 'self.observer is None but also in use?'
+            if self.observer is None:
+                raise RuntimeError("Observer was None but is it in use?")
             # Note: Uses undocumented attribute
             emitter = self.observed and self.observer._emitter_for_watch[self.observed]
 
@@ -555,7 +557,8 @@ class EDLogs(FileSystemEventHandler):
         try:
             # Preserve property order because why not?
             entry: MutableMapping[str, Any] = json.loads(line)
-            assert 'timestamp' in entry, "Timestamp does not exist in the entry"
+            if 'timestamp' not in entry:
+                raise KeyError("Timestamp does not exist in the entry")
 
             self.__navroute_retry()
 
@@ -1658,7 +1661,8 @@ class EDLogs(FileSystemEventHandler):
                                 self.state[category].pop(material)
 
                 module = self.state['Modules'][entry['Slot']]
-                assert module['Item'] == self.canonicalise(entry['Module'])
+                if module['Item'] != self.canonicalise(entry['Module']):
+                    raise ValueError(f"Module {entry['Slot']} is not {entry['Module']}")
                 module['Engineering'] = {
                     'Engineer':      entry['Engineer'],
                     'EngineerID':    entry['EngineerID'],

outfitting.py

@@ -59,7 +59,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
         moduledata.update(json.loads(modules_path.read_text()))
 
     if not module.get('name'):
-        raise AssertionError(f'{module["id"]}')
+        raise ValueError(f"Module with ID {module['id']} is missing a 'name' field")
 
     name = module['name'].lower().split('_')
     new = {'id': module['id'], 'symbol': module['name']}
@@ -69,7 +69,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
         # Armour is ship-specific, and ship names can have underscores
         ship_name, armour, armour_grade = module["name"].lower().rsplit("_", 2)[0:3]
         if ship_name not in ship_map:
-            raise AssertionError(f"Unknown ship: {ship_name}")
+            raise ValueError(f"Unknown ship: {ship_name}")
         new['category'] = 'standard'
         new["name"] = armour_map[armour_grade]
         new["ship"] = ship_map[ship_name]
@@ -115,7 +115,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
         new['category'] = 'utility'
         new['name'] = utility_map[name[1]]
         if not name[2].startswith('size') or not name[3].startswith('class'):
-            raise AssertionError(f'{module["id"]}: Unknown class/rating "{name[2]}/{name[3]}"')
+            raise ValueError(f'{module["id"]}: Unknown class/rating "{name[2]}/{name[3]}"')
 
         new['class'] = str(name[2][4:])
         new['rating'] = rating_map[name[3][5:]]
@@ -128,13 +128,13 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
             name[1] = f'{prefix}_{name[1]}'
 
         if name[1] not in weapon_map:
-            raise AssertionError(f'{module["id"]}: Unknown weapon "{name[0]}"')
+            raise ValueError(f'{module["id"]}: Unknown weapon "{name[0]}"')
 
         if name[2] not in weaponmount_map:
-            raise AssertionError(f'{module["id"]}: Unknown weapon mount "{name[2]}"')
+            raise ValueError(f'{module["id"]}: Unknown weapon mount "{name[2]}"')
 
         if name[3] not in weaponclass_map:
-            raise AssertionError(f'{module["id"]}: Unknown weapon class "{name[3]}"')
+            raise ValueError(f'{module["id"]}: Unknown weapon class "{name[3]}"')
 
         new['category'] = 'hardpoint'
         if len(name) > 4:
@@ -143,7 +143,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
                 new['rating'] = '?'
 
             elif '_'.join(name[:4]) not in weaponrating_map:
-                raise AssertionError(f'{module["id"]}: Unknown weapon rating "{module["name"]}"')
+                raise ValueError(f'{module["id"]}: Unknown weapon rating "{module["name"]}"')
 
             else:
                 # PP faction-specific weapons e.g. Hpt_Slugshot_Fixed_Large_Range
@@ -151,7 +151,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
                 new['rating'] = weaponrating_map['_'.join(name[:4])]  # assumes same rating as base weapon
 
         elif module['name'].lower() not in weaponrating_map:
-            raise AssertionError(f'{module["id"]}: Unknown weapon rating "{module["name"]}"')
+            raise ValueError(f'{module["id"]}: Unknown weapon rating "{module["name"]}"')
 
         else:
             new['name'] = weapon_map[name[1]]
@@ -165,7 +165,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
         new['class'] = weaponclass_map[name[3]]
 
     elif name[0] != 'int':
-        raise AssertionError(f'{module["id"]}: Unknown prefix "{name[0]}"')
+        raise ValueError(f'{module["id"]}: Unknown prefix "{name[0]}"')
 
     # Miscellaneous Class 1
     # e.g. Int_PlanetApproachSuite, Int_StellarBodyDiscoveryScanner_Advanced, Int_DockingComputer_Standard
@@ -207,7 +207,7 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
                 new['name'] = internal_map[len(name) > 4 and (name[1], name[4]) or name[1]]
 
         else:
-            raise AssertionError(f'{module["id"]}: Unknown module "{name[1]}"')
+            raise ValueError(f'{module["id"]}: Unknown module "{name[1]}"')
 
         if len(name) < 4 and name[1] == 'unkvesselresearch':  # Hack! No size or class.
             (new['class'], new['rating']) = ('1', 'E')
@@ -226,10 +226,10 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
 
         else:
             if len(name) < 3:
-                raise AssertionError(f'{name}: length < 3]')
+                raise ValueError(f'{name}: length < 3]')
 
             if not name[2].startswith('size') or not name[3].startswith('class'):
-                raise AssertionError(f'{module["id"]}: Unknown class/rating "{name[2]}/{name[3]}"')
+                raise ValueError(f'{module["id"]}: Unknown class/rating "{name[2]}/{name[3]}"')
 
             new['class'] = str(name[2][4:])
             new['rating'] = (name[1] == 'buggybay' and planet_rating_map or
@@ -258,10 +258,14 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
             print(f'No data for module {key}')
 
         elif new['name'] == 'Frame Shift Drive' or new['name'] == 'Frame Shift Drive (SCO)':
-            assert 'mass' in m and 'optmass' in m and 'maxfuel' in m and 'fuelmul' in m and 'fuelpower' in m, m
+            required_keys = ['mass', 'optmass', 'maxfuel', 'fuelmul', 'fuelpower']
+            missing_keys = [key for key in required_keys if key not in m]
 
+            if missing_keys:
+                raise KeyError(f"Missing keys: {', '.join(missing_keys)} in {m}")
         else:
-            assert 'mass' in m, m
+            if 'mass' not in m:
+                raise KeyError(f"Missing key 'mass' in {m}")
 
     new.update(moduledata.get(module['name'].lower(), {}))
 
@@ -269,10 +273,10 @@ def lookup(module, ship_map, entitled=False) -> dict | None:  # noqa: C901, CCR0
     mandatory_fields = ["id", "symbol", "category", "name", "class", "rating"]
     for field in mandatory_fields:
         if not new.get(field):
-            raise AssertionError(f'{module["id"]}: failed to set {field}')
+            raise ValueError(f'{module["id"]}: failed to set {field}')
 
     if new['category'] == 'hardpoint' and not new.get('mount'):
-        raise AssertionError(f'{module["id"]}: failed to set mount')
+        raise ValueError(f'{module["id"]}: failed to set mount')
 
     return new
 
@@ -284,8 +288,10 @@ def export(data, filename) -> None:
     :param data: CAPI data to export.
     :param filename: Filename to export into.
     """
-    assert "name" in data["lastSystem"]
-    assert "name" in data["lastStarport"]
+    if "name" not in data["lastSystem"]:
+        raise KeyError("Missing 'name' key in 'lastSystem'")
+    if "name" not in data["lastStarport"]:
+        raise KeyError("Missing 'name' key in 'lastStarport'")
 
     header = 'System,Station,Category,Name,Mount,Guidance,Ship,Class,Rating,FDevID,Date\n'
     rowheader = f'{data["lastSystem"]["name"]},{data["lastStarport"]["name"]}'
@@ -300,6 +306,6 @@ def export(data, filename) -> None:
                             f'{m.get("guidance","")}, {m.get("ship","")}, {m["class"]}, {m["rating"]},'
                             f'{m["id"]}, {data["timestamp"]}\n')
 
-            except AssertionError as e:
+            except ValueError as e:
                 # Log unrecognised modules
                 logger.debug('Outfitting', exc_info=e)

plug.py

@@ -113,20 +113,16 @@ class Plugin:
                 appitem = plugin_app(parent)
                 if appitem is None:
                     return None
-
                 if isinstance(appitem, tuple):
                     if (
                         len(appitem) != 2
                         or not isinstance(appitem[0], tk.Widget)
                         or not isinstance(appitem[1], tk.Widget)
                     ):
-                        raise AssertionError
-
+                        raise TypeError("Expected a tuple of two tk.Widget instances from plugin_app")
                 elif not isinstance(appitem, tk.Widget):
-                    raise AssertionError
-
+                    raise TypeError("Expected a tk.Widget or tuple of two tk.Widgets from plugin_app")
                 return appitem
-
             except Exception:
                 logger.exception(f'Failed for Plugin "{self.name}"')
 
@@ -148,7 +144,7 @@ class Plugin:
                 frame = plugin_prefs(parent, cmdr, is_beta)
                 if isinstance(frame, nb.Frame):
                     return frame
-                raise AssertionError
+                raise TypeError(f'Expected nb.Frame from plugin_prefs, got {type(frame).__name__}')
             except Exception:
                 logger.exception(f'Failed for Plugin "{self.name}"')
         return None
@@ -249,7 +245,8 @@ def invoke(
     for plugin in PLUGINS:
         if plugin.name == fallback:
             plugin_func = plugin._get_func(fn_name)
-            assert plugin_func, plugin.name  # fallback plugin should provide the function
+            if not plugin_func:
+                raise ValueError(f"Fallback plugin '{plugin.name}' does not provide the function '{fn_name}'")
             return plugin_func(*args)
 
     return None

protocol.py

@@ -67,7 +67,8 @@ if (config.auth_force_edmc_protocol  # noqa: C901
                 and not config.auth_force_localserver
         )):
     # This could be false if you use auth_force_edmc_protocol, but then you get to keep the pieces
-    assert sys.platform == 'win32'
+    if sys.platform != 'win32':
+        raise EnvironmentError("This code is for Windows only.")
     # spell-checker: words HBRUSH HICON WPARAM wstring WNDCLASS HMENU HGLOBAL
     from ctypes import (  # type: ignore
         windll, POINTER, WINFUNCTYPE, Structure, byref, c_long, c_void_p, create_unicode_buffer, wstring_at

scripts/find_localised_strings.py

@@ -260,7 +260,8 @@ def generate_lang_template(data: dict[pathlib.Path, list[ast.Call]]) -> str:
 """
     print(f"Done Deduping entries {len(entries)=}  {len(deduped)=}", file=sys.stderr)
     for entry in deduped:
-        assert len(entry.comments) == len(entry.locations)
+        if len(entry.comments) != len(entry.locations):
+            raise ValueError("Mismatch: 'comments' and 'locations' must have the same length.")
 
         comment_set = set()
         for comment, loc in zip(entry.comments, entry.locations):

shipyard.py

@@ -13,9 +13,12 @@ def export(data: companion.CAPIData, filename: str) -> None:
     :param filename: Optional filename to write to.
     :return:
     """
-    assert data['lastSystem'].get('name')
-    assert data['lastStarport'].get('name')
-    assert data['lastStarport'].get('ships')
+    if not data['lastSystem'].get('name'):
+        raise ValueError("Missing 'name' in 'lastSystem'")
+    if not data['lastStarport'].get('name'):
+        raise ValueError("Missing 'name' in 'lastStarport'")
+    if not data['lastStarport'].get('ships'):
+        raise ValueError("Missing 'ships' in 'lastStarport'")
 
     with open(filename, 'w', newline='') as f:
         c = csv.writer(f)

theme.py

@@ -146,7 +146,8 @@ class _Theme:
     def register(self, widget: tk.Widget | tk.BitmapImage) -> None:  # noqa: CCR001, C901
         # Note widget and children for later application of a theme. Note if
         # the widget has explicit fg or bg attributes.
-        assert isinstance(widget, (tk.BitmapImage, tk.Widget)), widget
+        if not isinstance(widget, (tk.Widget, tk.BitmapImage)):
+            raise TypeError(f'Expected widget, got {type(widget)}')
         if not self.defaults:
             # Can't initialise this til window is created       # Windows
             self.defaults = {
@@ -301,7 +302,8 @@ class _Theme:
         Also, register it for future updates.
         :param widget: Target widget.
         """
-        assert isinstance(widget, (tk.BitmapImage, tk.Widget)), widget
+        if not isinstance(widget, (tk.Widget, tk.BitmapImage)):
+            raise TypeError(f'Expected widget, got {type(widget)}')
         if not self.current:
             return  # No need to call this for widgets created in plugin_app()
 
@@ -325,7 +327,7 @@ class _Theme:
                 w_keys = []
 
             assert_str = f'{w_class} {widget} "{"text" in w_keys and widget["text"]}"'
-            raise AssertionError(assert_str)
+            raise ValueError(assert_str)
 
         attribs: set = self.widgets.get(widget, set())