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: Issue error and skip material on parse error #5217

Merged
merged 5 commits into from
Oct 1, 2024
Merged

Conversation

isaacwaldron
Copy link
Contributor

Issue a specific error message when parsing a material fails in import_materials_from_file. Also directly return False instead of relying on the catch-all exception handler.

Closes #5215

@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

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.36%. Comparing base (e658d50) to head (73ed060).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5217   +/-   ##
=======================================
  Coverage   83.35%   83.36%           
=======================================
  Files         142      142           
  Lines       58189    58193    +4     
=======================================
+ Hits        48506    48511    +5     
+ Misses       9683     9682    -1     

Copy link
Collaborator

@maxcapodi78 maxcapodi78 left a comment

Choose a reason for hiding this comment

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

stop_on_error is added but never used

src/ansys/aedt/core/modules/material_lib.py Outdated Show resolved Hide resolved
@isaacwaldron isaacwaldron marked this pull request as draft September 27, 2024 16:06
@isaacwaldron
Copy link
Contributor Author

Converted to draft to accommodate feedback on test mocking.

isaacwaldron and others added 5 commits September 29, 2024 17:18
Issue an error message when parsing a material fails in
import_materials_from_file.
In tests the error handler is disabled so we need to expect the
exception rather than checking for a False return value.
@isaacwaldron
Copy link
Contributor Author

@SMoraisAnsys @maxcapodi78 ready for review once again.

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 the test upgrade :)

@SMoraisAnsys
Copy link
Collaborator

@isaacwaldron There seem to be a problem with the changes introduced in #5203 making some of the CICD not running in this PR. I'll try to fix that asap.

Copy link
Contributor

@Alberto-DM Alberto-DM left a comment

Choose a reason for hiding this comment

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

LGTM

@Samuelopez-ansys Samuelopez-ansys enabled auto-merge (squash) October 1, 2024 07:31
@Samuelopez-ansys Samuelopez-ansys merged commit 78edb45 into main Oct 1, 2024
42 checks passed
@Samuelopez-ansys Samuelopez-ansys deleted the issue-5215 branch October 1, 2024 07:32
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.

Limited error handling in import_materials_from_file
5 participants