ansys / pyaedt

AEDT Python Client Package
https://aedt.docs.pyansys.com
MIT License
201 stars 120 forks source link

FEAT: Filtersolutions #4858

Closed ramin4667 closed 2 months ago

ramin4667 commented 3 months ago

This feature adds the Filtersolutions Python application to the PyAEDT.

ansys-reviewer-bot[bot] commented 3 months ago

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.34%. Comparing base (2343f2b) to head (0c2155b). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4858 +/- ## ========================================== - Coverage 83.54% 83.34% -0.20% ========================================== Files 118 119 +1 Lines 54196 54347 +151 ========================================== + Hits 45276 45296 +20 - Misses 8920 9051 +131 ```
SMoraisAnsys commented 3 months ago

Thanks for the contribution. This is my first passing of review for this PR, it's quite a big one !

@Samuelopez-ansys Can we wait for this PR until we remove the compatibility with IronPython ? @myoung301 @ramin4667 I don't have an installation of 2025.1 locally, did you run the tests ?

ramin4667 commented 3 months ago

Reviewer comments are addressed and all tests are passed locally except the version comparison, pending a DLL update.

FAILED _unittest/test_45_FilterSolutions/test_filter/test_dll_interface.py::TestClass::test_version - AssertionError: assert 'FilterSolutions API Version 2024 R1 (Beta)' == 'FilterSolutions API Version 2025 R1'

  - FilterSolutions API Version 2025 R1
  ?                                ^
  + FilterSolutions API Version 2024 R1 (Beta)
  ?                                ^   +++++++``

Also, the code coverage is tested locally with the following report:

> pyaedt\filtersolutions.py                                               33      1    97%   74
> pyaedt\filtersolutions_core\__init__.py                                 12      1    92%   41
> pyaedt\filtersolutions_core\attributes.py                              777      3    99%   804-805, 809
> pyaedt\filtersolutions_core\dll_interface.py                            64      3    95%   52, 56, 60
> pyaedt\filtersolutions_core\graph_setup.py                              53      0   100%
> pyaedt\filtersolutions_core\ideal_response.py                          165      0   100%
> pyaedt\filtersolutions_core\lumped_nodes_and_leads.py                  106      0   100%
> pyaedt\filtersolutions_core\lumped_parasitics.py                        97      0   100%
> pyaedt\filtersolutions_core\lumped_termination_impedance.py            142      0   100%
> pyaedt\filtersolutions_core\lumped_topology.py                         310      0   100%
> pyaedt\filtersolutions_core\multiple_bands_table.py                     56      0   100%
> pyaedt\filtersolutions_core\transmission_zeros.py                       76      0   100%
myoung301 commented 2 months ago

Hi @SMoraisAnsys, I see this is pending your review. Is there anything else that needs to be changed with this PR before it's ready to approve and merge?

myoung301 commented 2 months ago

@maxcapodi78 Thank you for the review. The FS API is available starting with 2025 R1 so the tests won't run unless that version or later is used. We've added the FilterSolutions-specific files and folders to the exclusion list. There is one common file that had a few lines added by this PR that doesn't pass the coverage check because the new lines aren't tested until 2025 R1. I think this is ok, but if you'd like us to add # pragma: no cover, let us know and we'll get that in a follow-up PR. We'll also look to address the documentation in another PR soon.