-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: add WorkflowTopology class and workflow_to_workflow_topology operator #1902
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1902 +/- ##
==========================================
+ Coverage 86.62% 88.27% +1.65%
==========================================
Files 83 88 +5
Lines 9973 10174 +201
==========================================
+ Hits 8639 8981 +342
+ Misses 1334 1193 -141 |
849d50f
to
4ff8c4e
Compare
@@ -481,6 +482,11 @@ def _type_to_output_method(self): | |||
self._api.operator_getoutput_as_any, | |||
lambda obj, type: any.Any(server=self._server, any_dpf=obj).cast(type), | |||
), | |||
( |
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.
@Matteo-Baussart-ANSYS can a Workflow also potentially take in or output a WorkflowTopology object?
Can an operator take a WorkflowTopology as input?
@@ -48,3 +48,19 @@ def _sort_supported_kwargs(bound_method, **kwargs): | |||
warnings.warn(txt) | |||
# Return the accepted arguments | |||
return kwargs_in | |||
|
|||
|
|||
def indent(text, subsequent_indent="", initial_indent=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.
@Matteo-Baussart-ANSYS this will need a docstring with description and parameters. Also typehinting?
@@ -953,6 +953,15 @@ def to_graphviz(self, path: Union[os.PathLike, str]): | |||
"""Saves the workflow to a GraphViz file.""" | |||
return self._api.work_flow_export_graphviz(self, str(path)) | |||
|
|||
def get_topology(self): |
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.
@Matteo-Baussart-ANSYS same, missing a docstring and typehinting
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
# SOFTWARE. | ||
|
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.
This will need a title, reference tag and description. You can find examples of that being done in other modules.
from ansys.dpf.core.custom_container_base import CustomContainerBase | ||
|
||
|
||
class DataConnection(CustomContainerBase): |
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.
@Matteo-Baussart-ANSYS again, missing docstrings and typehinting everywhere
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.
Some specific questions, but a general request is to add docstrings, module headers, typehinting, and improve coverage.
This adds the PyDPF support of the
WorkflowData
class andworkflow_to_workflow_topology
operator,