-
Notifications
You must be signed in to change notification settings - Fork 136
refactor: enhance sys method type hints and clean up return value #4099
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
Conversation
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
Reviewer's GuideRefactors sys method by adding explicit type hints for cmd and return type and ensures trailing newline is removed from returned output. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
@pyansys-ci-bot 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.
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.
Pull Request Overview
Refactors the sys
method by adding type hints and ensuring the returned output doesn’t end with a newline.
- Added
cmd: str
and-> str
type annotations tosys
- Updated return logic to strip a single trailing newline from the command output
Comments suppressed due to low confidence (2)
src/ansys/mapdl/core/mapdl_grpc.py:1475
- [nitpick] The docstring should mention that the returned string has a trailing newline removed. Document the newline-stripping behavior to keep consumers informed.
def sys(self, cmd: str, **kwargs) -> str:
src/ansys/mapdl/core/mapdl_grpc.py:1475
- [nitpick] The method name
sys
shadows Python's built-insys
module. Consider renaming to reduce confusion or explicitly document its purpose.
def sys(self, cmd: str, **kwargs) -> str:
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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4099 +/- ##
==========================================
+ Coverage 91.82% 91.85% +0.02%
==========================================
Files 187 187
Lines 15033 15033
==========================================
+ Hits 13804 13808 +4
+ Misses 1229 1225 -4 🚀 New features to boost your workflow:
|
Description
As the title.
Issue linked
NA but extracted from #1300.
Checklist
draft
if it is not ready to be reviewed yet.feat: adding new MAPDL command
)Summary by Sourcery
Refine the sys method by adding type hints and ensuring the returned output has no trailing newline
Enhancements: