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

FIX: Don't always remove AEDT project files #413

Merged
merged 3 commits into from
Apr 26, 2024
Merged

FIX: Don't always remove AEDT project files #413

merged 3 commits into from
Apr 26, 2024

Conversation

ghost
Copy link

@ghost ghost commented Apr 24, 2024

Remove AEDT project-related files on EDB open (e.g. file.aedt and/or file.aedt.lock for file.aedb) only if Edb constructor is run with isreadonly False and remove_existing_aedt True.

Fixes #412

Remove AEDT project-related files on EDB open (e.g. file.aedt and/or
file.aedt.lock for file.aedb) only if Edb constructor is run with
isreadonly False and remove_existing_aedt True.
@ghost ghost requested review from svandenb-dev and SMoraisAnsys April 24, 2024 17:29
@ghost ghost changed the title Don't always remove AEDT project files FIX: Don't always remove AEDT project files Apr 24, 2024
@ghost
Copy link
Author

ghost commented Apr 24, 2024

@svandenb-dev BTW it looks like PyAEDT already has some logic to handle an existing .aedt file and open it if you try to open an edb.def or .aedb folder: https://github.com/ansys/pyaedt/blob/87739a9ea3223ee8e57d3d7c6ca7569a4f18eefe/pyaedt/application/Design.py#L1122

I guess we should leave it to users to understand when they have to remove the AEDT file before opening a modified EDB (with the warning I'm introducing here), what do you think?

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.

Great update. Please, could you perform the minor changes suggested to make the code compliant with the changes you performed (shutil.rmtree to os.unlink).

tests/legacy/unit/test_edb.py Outdated Show resolved Hide resolved
tests/legacy/unit/test_edb.py Outdated Show resolved Hide resolved
tests/legacy/unit/test_edb.py Outdated Show resolved Hide resolved
tests/legacy/unit/test_edb.py Outdated Show resolved Hide resolved
tests/legacy/unit/test_edb.py Outdated Show resolved Hide resolved
svandenb-dev
svandenb-dev previously approved these changes Apr 26, 2024
Copy link
Collaborator

@svandenb-dev svandenb-dev left a comment

Choose a reason for hiding this comment

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

LGTM

Rename mocks to match mocked function names.

Co-authored-by: Sébastien Morais <[email protected]>
Missed renaming one use of a mock in the previous commit, causing a test
to fail.
@ghost ghost requested review from SMoraisAnsys and svandenb-dev April 26, 2024 08:54
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 doing the requested changes !

@SMoraisAnsys SMoraisAnsys merged commit 9f657b5 into main Apr 26, 2024
25 checks passed
@SMoraisAnsys SMoraisAnsys deleted the issue-412 branch April 26, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opening an EDB should not (always?) delete corresponding aedt project file
3 participants