diff --git a/doc/changelog.d/7931.miscellaneous.md b/doc/changelog.d/7931.miscellaneous.md new file mode 100644 index 000000000000..0564d6962d09 --- /dev/null +++ b/doc/changelog.d/7931.miscellaneous.md @@ -0,0 +1 @@ +Improve type hint in generics diff --git a/pyproject.toml b/pyproject.toml index 744697d3623a..838055015369 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -321,7 +321,16 @@ exclude = [ "src/ansys/aedt/core/application", "src/ansys/aedt/core/emit_core", "src/ansys/aedt/core/filtersolutions_core", - "src/ansys/aedt/core/generic", + "src/ansys/aedt/core/generic/configurations.py", + "src/ansys/aedt/core/generic/file_utils.py", + "src/ansys/aedt/core/generic/general_methods.py", + "src/ansys/aedt/core/generic/ibis_reader.py", + "src/ansys/aedt/core/generic/numbers_utils.py", + "src/ansys/aedt/core/generic/math_utils.py", + "src/ansys/aedt/core/generic/python_optimizers.py", + "src/ansys/aedt/core/generic/quaternion.py", + "src/ansys/aedt/core/generic/scheduler.py", + "src/ansys/aedt/core/generic/settings.py", "src/ansys/aedt/core/internal", "src/ansys/aedt/core/misc", "src/ansys/aedt/core/modeler", diff --git a/src/ansys/aedt/core/generic/constants.py b/src/ansys/aedt/core/generic/constants.py index 6d06e20969c0..b900369c945a 100644 --- a/src/ansys/aedt/core/generic/constants.py +++ b/src/ansys/aedt/core/generic/constants.py @@ -27,6 +27,8 @@ from enum import IntEnum from enum import auto import math +from typing import Callable +from typing import cast import warnings from ansys.aedt.core.generic.settings import settings @@ -56,6 +58,31 @@ SpeedOfLight = 299792458.0 """Value for speed of light.""" +# Type aliases for type checking and readability +UnitCallable = Callable[[float, bool], float] +UnitEntry = int | float | UnitCallable | tuple[UnitCallable] + +# ############################# Helper Functions for type checking ################################ + + +# TODO: This is required because AEDT_UNITS is not only a dictionary of str to float, but also +# contains callables for unit conversion. If we ever update AEDT_UNITS to be a dictionary of str to +# float only, we can rework ``unit_converter`` and remove this helper function. +def _to_unit_callable(unit_entry: UnitEntry) -> UnitCallable | None: + """Return a unit conversion callable when available. + + This helper primarily exists to narrow ``UnitEntry`` variants in a + type-checker-friendly way before calling conversion functions. + """ + if callable(unit_entry): + return cast(UnitCallable, unit_entry) + if isinstance(unit_entry, tuple) and len(unit_entry) == 1 and callable(unit_entry[0]): + return unit_entry[0] + return None + + +# ################################################################################################# + def db20(x: float, inverse: bool = True) -> float: """Convert db20 to decimal and vice versa. @@ -266,26 +293,33 @@ def unit_converter( if not input_is_list: values = [values] converted_values = [] + input_unit = AEDT_UNITS[unit_system][input_units] + output_unit = AEDT_UNITS[unit_system][output_units] + input_callable = _to_unit_callable(input_unit) + output_callable = _to_unit_callable(output_unit) for value in values: if unit_system == "Temperature": - value = AEDT_UNITS[unit_system][input_units](value, False) - value = AEDT_UNITS[unit_system][output_units](value, output_units != "kel") - elif not callable(AEDT_UNITS[unit_system][input_units]) and not callable( - AEDT_UNITS[unit_system][output_units] - ): - value = value * AEDT_UNITS[unit_system][input_units] / AEDT_UNITS[unit_system][output_units] - elif not callable(AEDT_UNITS[unit_system][input_units]) and callable( - AEDT_UNITS[unit_system][output_units] - ): - value = value * AEDT_UNITS[unit_system][input_units] - value = AEDT_UNITS[unit_system][output_units](value, True) - elif callable(AEDT_UNITS[unit_system][input_units]) and not callable( - AEDT_UNITS[unit_system][output_units] - ): - value = AEDT_UNITS[unit_system][input_units](value, False) / AEDT_UNITS[unit_system][output_units] + if input_callable is None or output_callable is None: + raise ValueError( + "Invalid temperature conversion configuration: Temperature units should map to callables" + ) + value = input_callable(value, False) + value = output_callable(value, output_units != "kel") + elif isinstance(input_unit, (int, float)) and isinstance(output_unit, (int, float)): + value = value * input_unit / output_unit + elif isinstance(input_unit, (int, float)) and output_callable is not None: + value = value * input_unit + value = output_callable(value, True) + elif input_callable is not None and isinstance(output_unit, (int, float)): + value = input_callable(value, False) / output_unit + elif input_callable is not None and output_callable is not None: + value = input_callable(value, False) + value = output_callable(value, True) + # NOTE: This should never happen but better to be safe than sorry. If this happens, it means that the + # recent changes to AEDT_UNITS have introduced a unit conversion configuration that is not supported + # by the current implementation of unit_converter. else: - value = AEDT_UNITS[unit_system][input_units](value, False) - value = AEDT_UNITS[unit_system][output_units](value, True) + raise ValueError(f"Invalid unit conversion configuration for unit system: {unit_system}") converted_values.append(value) if input_is_list: @@ -316,14 +350,16 @@ def scale_units(scale_to_unit: str) -> float: """ sunit = 1.0 + found_non_numeric = False for val in list(AEDT_UNITS.values()): for unit, scale_val in val.items(): if scale_to_unit.lower() == unit.lower(): - sunit = scale_val + if isinstance(scale_val, (int, float)): + return float(scale_val) + found_non_numeric = True break - else: - continue - break + if found_non_numeric: + warnings.warn(f"Unit '{scale_to_unit}' does not define a numeric scale factor") return sunit diff --git a/src/ansys/aedt/core/generic/data_handlers.py b/src/ansys/aedt/core/generic/data_handlers.py index 70bbddcb0968..add593ebfbbd 100644 --- a/src/ansys/aedt/core/generic/data_handlers.py +++ b/src/ansys/aedt/core/generic/data_handlers.py @@ -39,6 +39,23 @@ json_to_dict = read_json """Value for JSON to dict.""" +# ############################# Helper Functions for type checking ################################ + + +def _first_alpha_index(value: str) -> int | None: + """Return the first alphabetic index in a value string. + + This helper primarily exists to handle ``re.search`` optional results in a + type-checker-friendly way before parsing numeric/unit parts. + """ + loc = re.search("[a-zA-Z]", value) + if loc is None: + return None + return loc.span()[0] + + +# ################################################################################################# + @pyaedt_function_handler() def _dict_items_to_list_items(d, k, idx: str = "name") -> None: @@ -259,7 +276,7 @@ def format_decimals(el: float | int | str) -> str: @pyaedt_function_handler() -def random_string(length: int = 6, only_digits: bool = False, char_set: str = None) -> str: +def random_string(length: int = 6, only_digits: bool = False, char_set: str | None = None) -> str: """Generate a random string. Parameters @@ -664,11 +681,13 @@ def float_units(val_str: str, units: str = "") -> float: if units not in unit_val: raise Exception("Specified unit string " + units + " not known!") - loc = re.search("[a-zA-Z]", val_str) + first_alpha_index = _first_alpha_index(val_str) try: - b = loc.span()[0] - var = [float(val_str[0:b]), val_str[b:]] - val = var[0] * unit_val[var[1]] + if first_alpha_index is None: + raise ValueError + magnitude = float(val_str[0:first_alpha_index]) + unit_name = val_str[first_alpha_index:] + val = magnitude * unit_val[unit_name] except Exception: val = float(val_str) diff --git a/src/ansys/aedt/core/generic/design_types.py b/src/ansys/aedt/core/generic/design_types.py index 5ffcfdc55ff9..4c8df00d16b6 100644 --- a/src/ansys/aedt/core/generic/design_types.py +++ b/src/ansys/aedt/core/generic/design_types.py @@ -25,6 +25,7 @@ import re import sys import time +from typing import cast from ansys.aedt.core.circuit import Circuit from ansys.aedt.core.circuit_netlist import CircuitNetlist @@ -32,6 +33,9 @@ from ansys.aedt.core.generic.general_methods import is_linux from ansys.aedt.core.generic.protocols import AppType from ansys.aedt.core.generic.protocols import _AppWithOProject +from ansys.aedt.core.generic.protocols import _ODesign +from ansys.aedt.core.generic.protocols import _ODesktop +from ansys.aedt.core.generic.protocols import _OProject from ansys.aedt.core.generic.settings import settings from ansys.aedt.core.hfss import Hfss from ansys.aedt.core.hfss3dlayout import Hfss3dLayout @@ -53,6 +57,23 @@ Simplorer = TwinBuilder """Value for simplorer.""" +# ############################# Helper Functions for type checking ################################ + + +def _design_name_from_top_design_entry(design_entry: str) -> str | None: + """Extract the design name from an AEDT design entry string. + + This helper primarily exists to handle ``re.search`` optional results in a + type-checker-friendly way before using the match value. + """ + match = re.search(r"[^;]+$", design_entry) + if match is None: + return None + return match.group(0) + + +# ################################################################################################# + def launch_desktop( version: str | None = None, @@ -136,27 +157,32 @@ def launch_desktop( return d -app_map: dict[str, AppType | None] = { - "Maxwell 2D": Maxwell2d, - "Maxwell 3D": Maxwell3d, - "Maxwell Circuit": MaxwellCircuit, - "Twin Builder": TwinBuilder, - "Circuit Design": Circuit, - "Circuit Netlist": CircuitNetlist, - "2D Extractor": Q2d, - "Q3D Extractor": Q3d, - "HFSS": Hfss, - "Mechanical": Mechanical, - "IcepakFEA": Mechanical, - "Icepak": Icepak, - "Rmxprt": Rmxprt, - "HFSS 3D Layout Design": Hfss3dLayout, - "EMIT": Emit, -} +app_map: dict[str, AppType | None] = cast( + dict[str, AppType | None], + { + "Maxwell 2D": Maxwell2d, + "Maxwell 3D": Maxwell3d, + "Maxwell Circuit": MaxwellCircuit, + "Twin Builder": TwinBuilder, + "Circuit Design": Circuit, + "Circuit Netlist": CircuitNetlist, + "2D Extractor": Q2d, + "Q3D Extractor": Q3d, + "HFSS": Hfss, + "Mechanical": Mechanical, + "IcepakFEA": Mechanical, + "Icepak": Icepak, + "Rmxprt": Rmxprt, + "HFSS 3D Layout Design": Hfss3dLayout, + "EMIT": Emit, + }, +) """Value for app map.""" -def get_pyaedt_app(project_name: str = None, design_name: str = None, desktop: Desktop = None) -> _AppWithOProject: +def get_pyaedt_app( + project_name: str | None = None, design_name: str | None = None, desktop: Desktop | None = None +) -> _AppWithOProject | None: """Get the PyAEDT object with a given project name and design name. Parameters @@ -170,8 +196,8 @@ def get_pyaedt_app(project_name: str = None, design_name: str = None, desktop: D Returns ------- - :def :`ansys.aedt.core.Hfss` - Any of the PyAEDT App initialized. + _AppWithOProject | None + PyAEDT object with a given project name and design name or None if the design type is not supported. Examples -------- @@ -192,15 +218,20 @@ def get_pyaedt_app(project_name: str = None, design_name: str = None, desktop: D if project_name in list(desktop.project_list): odesktop = desktop.odesktop break + # NOTE: I think a ValueError is more appropriate here, but the original + # code used AttributeError in other places so we keep that error type for consistency. + if odesktop is None: + raise AttributeError(f"Project {project_name} doesn't exist in active desktop sessions.") elif _desktop_sessions: odesktop = list(_desktop_sessions.values())[-1].odesktop elif "oDesktop" in dir(sys.modules["__main__"]): # ironpython odesktop = sys.modules["__main__"].oDesktop # ironpython else: raise AttributeError("No Desktop Present.") + odesktop = cast(_ODesktop, odesktop) if not process_id: process_id = odesktop.GetProcessID() - if project_name and project_name not in desktop.project_list: + if project_name and desktop and project_name not in desktop.project_list: raise AttributeError(f"Project {project_name} doesn't exist in current desktop.") if not project_name: oProject = odesktop.GetActiveProject() @@ -211,25 +242,30 @@ def get_pyaedt_app(project_name: str = None, design_name: str = None, desktop: D odesktop.CloseAllWindows() if not oProject: raise AttributeError("No project is present.") + oproject = cast(_OProject, oProject) design_names = [] - deslist = list(oProject.GetTopDesignList()) + deslist = list(oproject.GetTopDesignList()) for el in deslist: - m = re.search(r"[^;]+$", el) - design_names.append(m.group(0)) + parsed_design_name = _design_name_from_top_design_entry(el) + if parsed_design_name is not None: + design_names.append(parsed_design_name) if design_name and design_name not in design_names: raise AttributeError(f"Design {design_name} doesn't exist in current project.") if not design_name: - oDesign = oProject.GetActiveDesign() + oDesign = oproject.GetActiveDesign() else: - oDesign = oProject.SetActiveDesign(design_name) + oDesign = oproject.SetActiveDesign(design_name) if is_linux and settings.aedt_version == "2024.1": # pragma: no cover time.sleep(1) odesktop.CloseAllWindows() if not oDesign: raise AttributeError("No design is present.") - design_type = oDesign.GetDesignType() + odesign = cast(_ODesign, oDesign) + design_type = odesign.GetDesignType() if design_type in app_map: version = odesktop.GetVersion().split(".") v = ".".join([version[0], version[1]]) - return app_map[design_type](project_name, design_name, version=v, aedt_process_id=process_id) + app_factory = app_map[design_type] + if app_factory is not None: + return app_factory(project_name, design_name, version=v, aedt_process_id=process_id) return None diff --git a/src/ansys/aedt/core/generic/protocols.py b/src/ansys/aedt/core/generic/protocols.py index a2c2631c6c42..ae732759092d 100644 --- a/src/ansys/aedt/core/generic/protocols.py +++ b/src/ansys/aedt/core/generic/protocols.py @@ -23,10 +23,21 @@ # SOFTWARE. from collections.abc import Callable +from typing import Iterable from typing import Protocol from typing import TypeAlias +class _ODesktop(Protocol): + """Interface definition for the AEDT desktop object.""" + + def GetProcessID(self) -> int: ... + def GetActiveProject(self) -> object: ... + def SetActiveProject(self, *args, **kwargs) -> object: ... + def CloseAllWindows(self) -> object: ... + def GetVersion(self) -> str: ... + + class _OProject(Protocol): """Interface definition for the AEDT project object.""" @@ -44,7 +55,7 @@ def GetDefinitionManager(self, *args, **kwargs) -> object: ... def GetDesigns(self) -> object: ... def GetName(self) -> object: ... def GetPath(self) -> object: ... - def GetTopDesignList(self) -> object: ... + def GetTopDesignList(self) -> Iterable[str]: ... def GetVariableValue(self, *args, **kwargs) -> object: ... def InsertDesign(self, *args, **kwargs) -> object: ... def Paste(self) -> object: ... @@ -54,6 +65,12 @@ def SaveAs(self, *args, **kwargs) -> object: ... def SetActiveDesign(self, *args, **kwargs) -> object: ... +class _ODesign(Protocol): + """Interface definition for the AEDT design object.""" + + def GetDesignType(self) -> str: ... + + class _AppWithOProject(Protocol): """Interface definition for PyAEDT app instances.""" diff --git a/tests/unit/test_geometry_operators.py b/tests/unit/test_geometry_operators.py index 59a13966c3bc..4b141dad78aa 100644 --- a/tests/unit/test_geometry_operators.py +++ b/tests/unit/test_geometry_operators.py @@ -23,9 +23,11 @@ # SOFTWARE. import math +from unittest.mock import NonCallableMock import pytest +from ansys.aedt.core.generic.constants import AEDT_UNITS from ansys.aedt.core.generic.constants import Axis from ansys.aedt.core.generic.constants import Plane from ansys.aedt.core.generic.constants import SweepDraft @@ -337,6 +339,30 @@ def test_unit_converter() -> None: assert unit_converter(10, "Power", "dBW", "W") == 10 +def test_unit_converter_temperature_invalid_configuration(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that unit_converter raises a ValueError when an invalid temperature conversion configuration is provided.""" + monkeypatch.setitem(AEDT_UNITS["Temperature"], "cel", 1.0) + + with pytest.raises(ValueError, match="Invalid temperature conversion configuration"): + unit_converter(10, "Temperature", "cel", "fah") + + +def test_unit_converter_invalid_configuration(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that unit_converter raises a ValueError when the unit configuration is invalid.""" + key = "DummyKey" + monkeypatch.setitem( + AEDT_UNITS, + key, + { + "input_unit": NonCallableMock(), + "output_unit": NonCallableMock(), + }, + ) + + with pytest.raises(ValueError, match=f"Invalid unit conversion configuration for unit system: {key}"): + unit_converter(10, key, "input_unit", "output_unit") + + def test_are_segments_intersecting() -> None: # crossing assert not go.are_segments_intersecting([1, 1], [10, 1], [1, 2], [10, 2])