Refact edb load amat #3805
Merged
Refact edb load amat #3805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
PipKat
reviewed
Oct 26, 2023
Codecov Report
@@ Coverage Diff @@
## main #3805 +/- ##
==========================================
- Coverage 81.13% 81.11% -0.02%
==========================================
Files 180 180
Lines 62314 62281 -33
==========================================
- Hits 50557 50521 -36
- Misses 11757 11760 +3 |
MaxJPRey
reviewed
Oct 28, 2023
MaxJPRey
reviewed
Oct 28, 2023
MaxJPRey
reviewed
Oct 28, 2023
MaxJPRey
reviewed
Oct 28, 2023
|
@maxcapodi78 I did the quick changes we spoke of. If you want me to rework something else, feel free to point it :) |
MaxJPRey
reviewed
Oct 28, 2023
MaxJPRey
reviewed
Oct 28, 2023
PipKat
reviewed
Oct 30, 2023
PipKat
reviewed
Oct 30, 2023
PipKat
reviewed
Oct 30, 2023
18 tasks
hui-zhou-a
approved these changes
Oct 31, 2023
Problem Current implementation leverages load_entire_aedt_file from pyaedt.generic.LoadAEDTFile. Solution The proposed solution consists in reworking the way amat are read and loaded. First, we only allow to load amat that are located in AEDT's syslib directory. By default, the file that is read is "Materials.amat". Since we only work with syslib, the code related to personallib has been removed. Note The proposed solutions is inspired from #3711. The resulting code refacts the PR's code to have fewers LOC and fixes many errors that were part of the PR.
Problem A warning is raised during workflow "Build project documentation". Solution Follow pydata/pydata-sphinx-theme#1492 instructions and disable "navigation_with_keys" as it is also something that is defined as default for ansys sphinx theme (see ansys/ansys-sphinx-theme#315)
SMoraisAnsys
force-pushed
the
refact/edb_load_amat
branch
from
e223656
to
b48ae86
Compare
|
@maxcapodi78 I think I've taken every feedback into account so I'll merge and close this PR. Feel free to contact me if I should do other modifications :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a proposition to refact how EDB reads materials file
Problem
Current implementation leverages load_entire_aedt_file from pyaedt.generic.LoadAEDTFile.
Solution
The proposed solution consists in reworking the way amat are read and loaded. First, we only allow to load amat that are located in AEDT's syslib directory. By default, the file that is read is "Materials.amat".
Since we only work with syslib, the code related to personallib has been removed.
Note
The proposed solutions is inspired from #3711. The resulting code refacts the PR's code to have fewers LOC and fixes many errors that were part of the PR.