From a5cbc5e91e098c1def8ddaec10b4edc19ce463b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Morais?= <146729917+SMoraisAnsys@users.noreply.github.com> Date: Fri, 10 Jan 2025 11:51:16 +0100 Subject: [PATCH] REFACTOR: Improve code quality and test coverage (#5642) Co-authored-by: Samuelopez-ansys --- .../aedt/core/generic/general_methods.py | 2 +- .../modeler/circuits/primitives_nexxim.py | 90 +++++++++++-------- src/ansys/aedt/core/modules/solve_setup.py | 5 +- tests/unit/conftest.py | 15 ++-- tests/unit/test_primitives_nexxim.py | 51 +++++++++++ tests/unit/test_warnings.py | 2 +- 6 files changed, 116 insertions(+), 49 deletions(-) create mode 100644 tests/unit/test_primitives_nexxim.py diff --git a/src/ansys/aedt/core/generic/general_methods.py b/src/ansys/aedt/core/generic/general_methods.py index 560ce0aa529..5bdf80d4933 100644 --- a/src/ansys/aedt/core/generic/general_methods.py +++ b/src/ansys/aedt/core/generic/general_methods.py @@ -404,7 +404,7 @@ def open_file(file_path, file_options="r", encoding=None, override_existing=True Parameters ---------- - file_path : str + file_path : str or Path Full absolute path to the file (either local or remote). file_options : str, optional Options for opening the file. diff --git a/src/ansys/aedt/core/modeler/circuits/primitives_nexxim.py b/src/ansys/aedt/core/modeler/circuits/primitives_nexxim.py index 8494f0ff950..284a623e6e7 100644 --- a/src/ansys/aedt/core/modeler/circuits/primitives_nexxim.py +++ b/src/ansys/aedt/core/modeler/circuits/primitives_nexxim.py @@ -22,7 +22,7 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -import os +from pathlib import Path import re import secrets import time @@ -314,8 +314,9 @@ def connect_components_in_series(self, assignment, use_wire=True): if component in [cmp.id, cmp.name, cmp.composed_name]: comps.append(cmp) break + if len(comps) < 2: + raise RuntimeError("At least two components have to be passed.") i = 0 - assert len(comps) > 1, "At least two components have to be passed." while i < (len(comps) - 1): comps[i].pins[-1].connect_to_component(comps[i + 1].pins[0], use_wire=use_wire) i += 1 @@ -352,7 +353,8 @@ def connect_components_in_parallel(self, assignment): if component in [cmp.id, cmp.name, cmp.composed_name]: comps.append(cmp) break - assert len(comps) > 1, "At least two components have to be passed." + if len(comps) < 2: + raise RuntimeError("At least two components have to be passed.") comps[0].pins[0].connect_to_component([i.pins[0] for i in comps[1:]]) terminal_to_connect = [cmp for cmp in comps if len(cmp.pins) >= 2] if len(terminal_to_connect) > 1: @@ -1616,14 +1618,14 @@ def _add_subcircuit_link( Name of the subcircuit HFSS link. pin_names : list List of the pin names. - source_project_path : str + source_project_path : str or Path Path to the source project. source_design_name : str Name of the design. solution_name : str, optional Name of the solution and sweep. The default is ``"Setup1 : Sweep"``. - image_subcircuit_path : str, optional + image_subcircuit_path : str or Path or None Path of the Picture used in Circuit. Default is an HFSS Picture exported automatically. model_type : str, optional @@ -1647,6 +1649,8 @@ def _add_subcircuit_link( >>> oComponentManager.Add >>> oDesign.AddCompInstance """ + if isinstance(source_project_path, str): + source_project_path = Path(source_project_path) model = "hfss" owner = "HFSS" icon_file = "hfss.bmp" @@ -1668,19 +1672,20 @@ def _add_subcircuit_link( hspice_customization = self._get_comp_custom_settings(3, 1, 2, 3, 0, 0, "False", "", 3) if image_subcircuit_path: - _, file_extension = os.path.splitext(image_subcircuit_path) - if file_extension != ".gif" or file_extension != ".bmp" or file_extension != ".jpg": + if isinstance(image_subcircuit_path, str): + image_subcircuit_path = Path(image_subcircuit_path) + if image_subcircuit_path.suffix not in [".gif", ".bmp", ".jpg"]: image_subcircuit_path = None warnings.warn("Image extension is not valid. Use default image instead.") if not image_subcircuit_path: - image_subcircuit_path = os.path.normpath( - os.path.join(self._modeler._app.desktop_install_dir, "syslib", "Bitmaps", icon_file) - ) + image_subcircuit_path = ( + Path(self._modeler._app.desktop_install_dir) / "syslib" / "Bitmaps" / icon_file + ).resolve() filename = "" comp_name_aux = generate_unique_name(source_design_name) WB_SystemID = source_design_name - if not self._app.project_file == source_project_path: - filename = source_project_path + if Path(self._app.project_file) != source_project_path: + filename = str(source_project_path) comp_name_aux = comp_name WB_SystemID = "" @@ -1699,7 +1704,7 @@ def _add_subcircuit_link( "Description:=", "", "ImageFile:=", - image_subcircuit_path, + str(image_subcircuit_path), "SymbolPinConfiguration:=", 0, ["NAME:PortInfoBlk"], @@ -1946,7 +1951,7 @@ def set_sim_solution_on_hfss_subcircuit(self, component, solution_name="Setup1 : @pyaedt_function_handler() def _edit_link_definition_hfss_subcircuit(self, component, edited_prop): - """Generic function to set the link definition for an hfss subcircuit.""" + """Generic function to set the link definition for a HFSS subcircuit.""" if isinstance(component, str): complist = component.split(";") elif isinstance(component, CircuitComponent): @@ -1955,16 +1960,21 @@ def _edit_link_definition_hfss_subcircuit(self, component, edited_prop): complist = self.components[component].composed_name.split(";") else: raise AttributeError("Wrong Component Input") - complist2 = complist[0].split("@") - arg = ["NAME:AllTabs"] - arg1 = ["NAME:Model"] - arg2 = ["NAME:PropServers", "Component@" + str(complist2[1])] - arg3 = ["NAME:ChangedProps", edited_prop] - - arg1.append(arg2) - arg1.append(arg3) - arg.append(arg1) + arg = [ + "NAME:AllTabs", + [ + "NAME:Model", + [ + "NAME:PropServers", + "Component@" + str(complist[0].split("@")[1]), + ], + [ + "NAME:ChangedProps", + edited_prop, + ], + ], + ] self._app._oproject.ChangeProperty(arg) return True @@ -2016,7 +2026,7 @@ def create_component_from_spicemodel( Parameters ---------- - input_file : str + input_file : str or Path Path to .lib file. model : str, optional Model name to import. If `None` the first subckt in the lib file will be placed. @@ -2038,12 +2048,15 @@ def create_component_from_spicemodel( Examples -------- + >>> from pathlib import Path >>> from ansys.aedt.core import Circuit >>> cir = Circuit(version="2023.2") - >>> model = os.path.join("Your path", "test.lib") + >>> model = Path("Your path") / "test.lib" >>> cir.modeler.schematic.create_component_from_spicemodel(input_file=model,model="GRM1234",symbol="nexx_cap") >>> cir.release_desktop(False, False) """ + if isinstance(input_file, str): + input_file = Path(input_file) models = self._parse_spice_model(input_file) if not model and models: model = models[0] @@ -2061,12 +2074,11 @@ def create_component_from_spicemodel( else: arg2.append([False, "", "", False]) arg.append(arg2) - self.o_component_manager.ImportModelsFromFile(input_file.replace("\\", "/"), arg) + self.o_component_manager.ImportModelsFromFile(input_file.as_posix(), arg) if create_component: return self.create_component(None, component_library=None, component_name=model, location=location) - else: - return True + return True @pyaedt_function_handler(model_path="input_file", solution_name="solution") def add_siwave_dynamic_link(self, input_file, solution=None, simulate_solutions=False): @@ -2074,7 +2086,7 @@ def add_siwave_dynamic_link(self, input_file, solution=None, simulate_solutions= Parameters ---------- - input_file : str + input_file : str or Path Full path to the .siw file. solution : str, optional Solution name. @@ -2086,18 +2098,20 @@ def add_siwave_dynamic_link(self, input_file, solution=None, simulate_solutions= :class:`ansys.aedt.core.modeler.circuits.object_3d_circuit.CircuitComponent` Circuit Component Object. """ - assert os.path.exists(input_file), "Project file doesn't exist" - comp_name = os.path.splitext(os.path.basename(input_file))[0] - results_path = input_file + "averesults" - solution_path = os.path.join(results_path, comp_name + ".asol") - # out = load_entire_aedt_file(solution) + if isinstance(input_file, str): + input_file = Path(input_file) + if not input_file.exists(): + raise FileNotFoundError(f"Project file '{input_file}' doesn't exist") + comp_name = Path(input_file).stem + results_path = input_file.parent / f"{comp_name}.siwaveresults" + solution_path = results_path / f"{comp_name}.asol" out = load_keyword_in_aedt_file(solution_path, "Solutions") if not solution: solution = list(out["Solutions"]["SYZSolutions"].keys())[0] - results_folder = os.path.join( - results_path, - out["Solutions"]["SYZSolutions"][solution]["DiskName"], - out["Solutions"]["SYZSolutions"][solution]["DiskName"] + ".syzinfo", + results_folder = ( + results_path + / out["Solutions"]["SYZSolutions"][solution]["DiskName"] + / f"{out['Solutions']['SYZSolutions'][solution]['DiskName']}.syzinfo" ) pin_names = [] diff --git a/src/ansys/aedt/core/modules/solve_setup.py b/src/ansys/aedt/core/modules/solve_setup.py index 61987fe02e0..665765ff531 100644 --- a/src/ansys/aedt/core/modules/solve_setup.py +++ b/src/ansys/aedt/core/modules/solve_setup.py @@ -1892,11 +1892,12 @@ def solver_type(self): try: return self.properties["Solver"] except Exception: - pass + self.p_app.logger.debug("Cannot retrieve solver type with key 'Solver'") try: return self.props["SolveSetupType"] except Exception: - return None + self.p_app.logger.debug("Cannot retrieve solver type with key 'SolveSetupType'") + return None @pyaedt_function_handler() def create(self): diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index b47473860d7..3f4fd0841a1 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -25,15 +25,16 @@ from unittest.mock import MagicMock from unittest.mock import patch +from ansys.aedt.core.generic.settings import settings +from ansys.aedt.core.maxwell import Maxwell3d import pytest +settings.enable_error_handler = False + @pytest.fixture def maxwell_3d_setup(): - """Fixture used to setup a Maxwell3d instance.""" - with patch("ansys.aedt.core.maxwell.FieldAnalysis3D") as mock_fiel_analysis_3d, patch( - "ansys.aedt.core.maxwell.Maxwell" - ) as mock_maxwell: - mock_fiel_analysis_3d.return_value = MagicMock() - mock_maxwell.return_value = MagicMock() - yield {"FieldAnalysis3D": mock_fiel_analysis_3d, "Maxwell": mock_maxwell} + """Fixture used to mock the creation of a Maxwell instance.""" + with patch("ansys.aedt.core.maxwell.Maxwell3d.__init__", lambda x: None): + mock_instance = MagicMock(spec=Maxwell3d) + yield mock_instance diff --git a/tests/unit/test_primitives_nexxim.py b/tests/unit/test_primitives_nexxim.py new file mode 100644 index 00000000000..830c53ed349 --- /dev/null +++ b/tests/unit/test_primitives_nexxim.py @@ -0,0 +1,51 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2021 - 2025 ANSYS, Inc. and/or its affiliates. +# SPDX-License-Identifier: MIT +# +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +from pathlib import Path +from unittest.mock import MagicMock +from unittest.mock import patch +import warnings + +from ansys.aedt.core.modeler.circuits.primitives_nexxim import NexximComponents +import pytest + + +def test_add_siwave_dynamic_link_failure_with_file_not_found(): + file = Path("dummy") + modeler = MagicMock() + nexxim = NexximComponents(modeler) + + with pytest.raises(FileNotFoundError): + nexxim.add_siwave_dynamic_link(file) + + +@patch.object(warnings, "warn") +def test_add_subcircuit_link_warning_call(mock_warn): + file = Path("dummy") + modeler = MagicMock() + nexxim = NexximComponents(modeler) + + nexxim._add_subcircuit_link("dummy_comp", ["dummy_pin"], file, "dummy_source_design", image_subcircuit_path=file) + + mock_warn.assert_any_call("Image extension is not valid. Use default image instead.") diff --git a/tests/unit/test_warnings.py b/tests/unit/test_warnings.py index 8711af96542..ee2b487bf19 100644 --- a/tests/unit/test_warnings.py +++ b/tests/unit/test_warnings.py @@ -62,7 +62,7 @@ def test_deprecation_warning_with_valid_python_version(mock_warn, monkeypatch): def test_alias_deprecation_warning(): - """Test that pyaedt alias warning is triggered.""" + """Test that pyaedt alias warning is triggered.""" import importlib import pyaedt