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.

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

REFACT: Fix Circuit in linux 2024 R1 #4589

Merged
merged 20 commits into from
Apr 30, 2024
Merged

Conversation

Samuelopez-ansys
Copy link
Member

Add a common method for active project and active design

@ansys-reviewer-bot
Copy link
Contributor

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

@ansys-reviewer-bot review

@github-actions github-actions bot added the bug Something isn't working label Apr 25, 2024
@Samuelopez-ansys
Copy link
Member Author

@maxcapodi78 I defined active_design at desktop.py level because it is also used at this level, if you think it does not make sense, we can discuss.

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2024

Codecov Report

Attention: Patch coverage is 62.50000% with 45 lines in your changes are missing coverage. Please review.

Project coverage is 81.51%. Comparing base (7104268) to head (ef93eb0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4589      +/-   ##
==========================================
- Coverage   81.51%   81.51%   -0.01%     
==========================================
  Files         110      110              
  Lines       53772    53841      +69     
==========================================
+ Hits        43834    43887      +53     
- Misses       9938     9954      +16     

Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

@Samuelopez-ansys Just wondering if there is a way to avoid the latency that such changes will add.
For example, the property oeditor now contains

                if is_linux and settings.aedt_version == "2024.1":
                    time.sleep(1)
                    self._odesktop.CloseAllWindows()

I've briefly looked into our code base and it seems that a lot of classes tend to access this oeditor property. Could we have some cache mechanism to know that all windows are already closed and avoid this time.sleep(1) call ?

@Samuelopez-ansys
Copy link
Member Author

@SMoraisAnsys I did not find a method in the AEDT API to get this information... it is impacting only Linux in 2024.R1. Ansys development team is checking how to fix this in issue in 2024R2. So we need to decide if we want Circuit in 2024R1 in Linux, or not :)

@Samuelopez-ansys Samuelopez-ansys marked this pull request as draft April 25, 2024 12:36
SMoraisAnsys
SMoraisAnsys previously approved these changes Apr 25, 2024
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

LGTM, I proposed a few changes though

_unittest/test_21_Circuit.py Outdated Show resolved Hide resolved
_unittest/test_21_Circuit.py Outdated Show resolved Hide resolved
_unittest/test_21_Circuit.py Outdated Show resolved Hide resolved
_unittest/test_21_Circuit.py Outdated Show resolved Hide resolved
pyaedt/desktop.py Outdated Show resolved Hide resolved
Co-authored-by: Sébastien Morais <146729917+SMoraisAnsys@users.noreply.github.com>
Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
@Samuelopez-ansys Samuelopez-ansys marked this pull request as ready for review April 26, 2024 10:29
SMoraisAnsys
SMoraisAnsys previously approved these changes Apr 26, 2024
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for handling the Linux issues and making it possible to work with Circuit !

@SMoraisAnsys SMoraisAnsys merged commit 96dcbb4 into main Apr 30, 2024
15 checks passed
@SMoraisAnsys SMoraisAnsys deleted the fix/circuit_linux_2024R1 branch April 30, 2024 07:10
SMoraisAnsys added a commit that referenced this pull request Apr 30, 2024
SMoraisAnsys added a commit that referenced this pull request May 2, 2024
Seems like #4589 did not handle this tests and that it might
be worth it to reopen #4590. The skipped tests are currently
blocking #4284 from passing CICD as tests would be performed
on Linux.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants