-
Notifications
You must be signed in to change notification settings - Fork 128
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
added "DecadeCount" "OctaveCount" "ExponentialCount" in parametric sw… #3910
Conversation
…eep, removed unit from "step"
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
It seems that "LogScale" is actually not used at all and could be removed
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3910 +/- ##
=======================================
Coverage 81.35% 81.35%
=======================================
Files 182 182
Lines 63147 63151 +4
=======================================
+ Hits 51372 51377 +5
+ Misses 11775 11774 -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IreneWoyna Great implementation, but you need to add some unit test to pass the CodeCov:
It is not your fault, it was not covered before, but now you touched this part of the code and the bot detected it :) You just need to add these different sweeps in the unittest.
@IreneWoyna I'd remove the LogScale and add unit test to cover all new cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…eep, removed unit from "step"