-
Notifications
You must be signed in to change notification settings - Fork 23
refactor: add typing for workflow record function #2479
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
base: main
Are you sure you want to change the base?
Conversation
3a360d7
to
cfc74ac
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2479 +/- ##
==========================================
+ Coverage 84.12% 84.15% +0.02%
==========================================
Files 91 91
Lines 10703 10704 +1
==========================================
+ Hits 9004 9008 +4
+ Misses 1699 1696 -3 |
cfc74ac
to
466c945
Compare
62405d0
to
4ab8c8f
Compare
src/ansys/dpf/core/workflow.py
Outdated
@@ -624,17 +631,19 @@ def record(self, identifier="", transfer_ownership=True): | |||
return self._api.work_flow_record_instance(self, identifier, transfer_ownership) | |||
|
|||
@staticmethod | |||
def get_recorded_workflow(id, server=None): | |||
def get_recorded_workflow(id: int, server: AnyServerType | None = None) -> Workflow: |
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.
I do not think the union with None
is mandatory. I think it knows from the default value that the parameter is optional and defaults to None
.
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.
From experience, it doesn't always infers this. Maybe it does. Though, type checkers tend to error or warn about that, so I usually specify it may explicitly be None
.
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
Co-authored-by: Paul Profizi <[email protected]>
No description provided.