ansys / pyaedt

AEDT Python Client Package
https://aedt.docs.pyansys.com
MIT License
201 stars 120 forks source link

FEAT: Enable harmonic force #4848

Closed JuliusSaitz closed 1 day ago

ansys-reviewer-bot[bot] commented 3 months ago

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

@ansys-reviewer-bot review

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.64%. Comparing base (a6445bb) to head (84054f4). Report is 128 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4848 +/- ## =========================================== - Coverage 83.46% 72.64% -10.82% =========================================== Files 120 120 Lines 54562 54562 =========================================== - Hits 45540 39638 -5902 - Misses 9022 14924 +5902 ```
SMoraisAnsys commented 3 months ago

@JuliusSaitz Thanks a lot for opening this PR. We haven't had the time to work on #4779 so it's really great that you are dealing with it. We are working on refactoring our codebase to deprecate some argument names so this PR will have to follow this and keep use of assignment instead of object.

Pinging @Samuelopez-ansys and @gmalinve for feedback on the changes

SMoraisAnsys commented 3 months ago

I took the liberty to "commit" by merging the main branch. Doing so will allow your PR to run the tests correctly.

gmalinve commented 3 months ago

@JuliusSaitz good job! thanks for your contribution! Now you should add some UT to cover the new functionalities you implemented :)

SMoraisAnsys commented 3 months ago

@JuliusSaitz Could you please add some tests to your PR ? We enforce new development to be tested to avoid adding failing code to the main branch and also to avoid adding bugs in the future.

If needed we can help you with the argument renaming afterward.

JuliusSaitz commented 3 months ago

Hi Sebastien,

Yes, I understand, and I will. It's just a matter of time. Right now, I need to concentrate on other work. Thanks.

Best regards

Julius


From: Sébastien Morais @.> Sent: Tuesday, July 2, 2024 11:27 To: ansys/pyaedt @.> Cc: Julius Saitz (External) @.>; Mention @.> Subject: Re: [ansys/pyaedt] FEAT: Enable harmonic force (PR #4848)

[External Sender]

@JuliusSaitzhttps://github.com/JuliusSaitz Could you please add some tests to your PR ? We enforce new development to be tested to avoid adding failing code to the main branch and also to avoid adding bugs in the future.

If needed we can help you with the argument renaming afterward.

— Reply to this email directly, view it on GitHubhttps://github.com/ansys/pyaedt/pull/4848#issuecomment-2202543471, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AXITYQ523VPQ3MTK6JOHAZ3ZKJXADAVCNFSM6AAAAABJ3LIPDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBSGU2DGNBXGE. You are receiving this because you were mentioned.Message ID: @.***>

SMoraisAnsys commented 3 months ago

Awesome, just wanted to know if you had the will and time to continue the changes :)

SMoraisAnsys commented 1 month ago

Hi @JuliusSaitz, do you think you'll have time in the comings days/weeks to add the missing tests ? Thanks again for your contribution !

JuliusSaitz commented 1 month ago

Hi Sebastien,

It is on my to do list. I am in this week, but I have other things to finish of higher priority. If I manage, I will work on it, if not, it would have to wait until September. But as I once started it, I would certainly want to finish it. Thank you.

Best regards,

Julius


From: Sébastien Morais @.> Sent: Monday, August 12, 2024 14:08 To: ansys/pyaedt @.> Cc: Julius Saitz (External) @.>; Mention @.> Subject: Re: [ansys/pyaedt] FEAT: Enable harmonic force (PR #4848)

[External Sender]

Hi @JuliusSaitzhttps://github.com/JuliusSaitz, do you think you'll have time in the comings days/weeks to add the missing tests ? Thanks again for your contribution !

— Reply to this email directly, view it on GitHubhttps://github.com/ansys/pyaedt/pull/4848#issuecomment-2283796686, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AXITYQ2NJCMXJSGTPKALSSLZRCQSHAVCNFSM6AAAAABJ3LIPDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBTG44TMNRYGY. You are receiving this because you were mentioned.Message ID: @.***>

Samuelopez-ansys commented 2 days ago

Hi @JuliusSaitz ,

What is the status of this PR? Please could you finish it? If not we will close the PR without merging.

Thanks!

JuliusSaitz commented 1 day ago

Hi Samuel,

I really wanted to do this, but the situation with other activities is not really any better. Please, close this without merging. At one point I may return to this activity. Thanks.

Julius


From: Samuel Lopez @.> Sent: Tuesday, October 1, 2024 14:31 To: ansys/pyaedt @.> Cc: Julius Saitz (External) @.>; Mention @.> Subject: Re: [ansys/pyaedt] FEAT: Enable harmonic force (PR #4848)

[External Sender]

Hi @JuliusSaitzhttps://github.com/JuliusSaitz ,

What is the status of this PR? Please could you finish it? If not we will close the PR without merging.

Thanks!

— Reply to this email directly, view it on GitHubhttps://github.com/ansys/pyaedt/pull/4848#issuecomment-2385653401, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AXITYQ2AFWXJNFYLYWC5E5TZZKI27AVCNFSM6AAAAABJ3LIPDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBVGY2TGNBQGE. You are receiving this because you were mentioned.Message ID: @.***>