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

Enhancement/temp dep&transient bc #4062

Merged
merged 72 commits into from Mar 11, 2024

Conversation

lorenzovecchietti
Copy link
Collaborator

@lorenzovecchietti lorenzovecchietti commented Jan 11, 2024

Add classes to ease the creation of temperature dependent and transient boundary assignments.

Until now, the creation of a boundary condition with advanced conditions (transient or temperature dependent) was not immediate:

  1. each function contained a description of how to build a dictionary and with which keys and values it could function (repetitive, difficult to read and in any case not complete)
  2. no check whether there were all the necessary values

With this PR I introduce a base class (and subclasses) to handle all this easily.

@Samuelopez-ansys @maxcapodi78 let me know if you have suggestions

@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

@lorenzovecchietti lorenzovecchietti added the enhancement New features or code improvements label Jan 11, 2024
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3a6c2aa) 31.12% compared to head (b254e9d) 81.97%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4062       +/-   ##
===========================================
+ Coverage   31.12%   81.97%   +50.84%     
===========================================
  Files         182      182               
  Lines       63851    63968      +117     
===========================================
+ Hits        19875    52435    +32560     
+ Misses      43976    11533    -32443     

@lorenzovecchietti lorenzovecchietti marked this pull request as ready for review January 16, 2024 09:20
pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/modules/Boundary.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Feb 12, 2024

Codecov Report

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

Project coverage is 76.64%. Comparing base (10b617f) to head (730f53c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4062      +/-   ##
==========================================
- Coverage   81.51%   76.64%   -4.88%     
==========================================
  Files         107      107              
  Lines       52366    52485     +119     
==========================================
- Hits        42687    40225    -2462     
- Misses       9679    12260    +2581     

@lorenzovecchietti lorenzovecchietti enabled auto-merge (squash) February 13, 2024 15:40
pyaedt/modules/Boundary.py Outdated Show resolved Hide resolved
lorenzovecchietti and others added 2 commits February 20, 2024 15:10
Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
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 work ! Could you do a few changes to make this code compliant with pyaedt code base (cf discussion over returning False instead of raising Exception).

Note: I still strongly think we should move from return a boolean to raising an exception !

pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/icepak.py Outdated Show resolved Hide resolved
pyaedt/icepak.py Outdated Show resolved Hide resolved
_unittest/test_98_Icepak.py Outdated Show resolved Hide resolved
pyaedt/modules/Boundary.py Show resolved Hide resolved
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, good work !

@lorenzovecchietti lorenzovecchietti merged commit e3d98ca into main Mar 11, 2024
15 checks passed
@lorenzovecchietti lorenzovecchietti deleted the Enhancement/temp_dep&transient_bc branch March 11, 2024 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants