Skip to content

Commit

Permalink
Fix incorrect data node rendering for modular pipelines (#1439)
Browse files Browse the repository at this point in the history
The viz flowchart is actually a representation of modular pipelines. The root modular pipeline is the first thing we see in the collapsed flowchart. It must include:

All top-level modular pipelines (e.g., level1.node, not level2.level1.node).
Any tasks, datasets, or parameters not part of a modular pipeline.
Inputs and outputs to the top-level modular pipeline (also called external_inputs and external_outputs in the code).
Lastly, all parameters, even if they belong to nested modular pipelines. The flowchart design ensures these parameters remain visible from the root modular pipeline, even when we expand the view.
In the code, there was a variable named root_children_ids, which was causing issues as reported by users. It's a bit confusing what it does but removing it resulted in the parameters not being shown at the root level anymore.

To address this, the variable root_children_ids has been renamed to root_parameters, and it has been modified to include all parameters that are part of a modular pipeline.

After making this change, the code correctly satisfies the above four conditions, ensuring all relevant components are present in the root modular pipeline.
  • Loading branch information
rashidakanchwala authored Jul 25, 2023
1 parent f635564 commit 475a9c8
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 12 deletions.
5 changes: 5 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Please follow the established format:
- Include the ID number for the related PR (or PRs) in parentheses
-->


## Bug fixes and other changes

- Fix incorrect rendering of datasets in modular pipelines. (#1439)

# Release 6.3.4

## Bug fixes and other changes
Expand Down
16 changes: 4 additions & 12 deletions package/kedro_viz/data_access/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ def create_modular_pipelines_tree_for_registered_pipeline(
modular_pipelines_tree = modular_pipelines_services.expand_tree(
modular_pipelines.as_dict()
)
root_children_ids = set()
root_parameters = set()

# turn all modular pipelines in the tree into a graph node for visualisation,
# except for the artificial root node
Expand All @@ -397,32 +397,24 @@ def create_modular_pipelines_tree_for_registered_pipeline(
)

# only keep the modular pipeline's inputs belonging to the current registered pipeline
inputs_in_registered_pipeline = set()
for input_id in modular_pipeline_node.inputs:
input_node = self.nodes.get_node_by_id(input_id)
if input_node.belongs_to_pipeline(registered_pipeline_id):
edges.add_edge(
GraphEdge(source=input_id, target=modular_pipeline_id)
)
node_dependencies[input_id].add(modular_pipeline_id)
inputs_in_registered_pipeline.add(input_id)
root_children_ids.update(
modular_pipeline_node.external_inputs & inputs_in_registered_pipeline
)
if isinstance(input_node, ParametersNode):
root_parameters.add(input_id)

# only keep the modular pipeline's outputs belonging to the current registered pipeline
outputs_in_registered_pipeline = set()
for output_id in modular_pipeline_node.outputs:
output_node = self.nodes.get_node_by_id(output_id)
if output_node.belongs_to_pipeline(registered_pipeline_id):
edges.add_edge(
GraphEdge(source=modular_pipeline_id, target=output_id)
)
node_dependencies[modular_pipeline_id].add(output_id)
outputs_in_registered_pipeline.add(output_id)
root_children_ids.update(
modular_pipeline_node.external_outputs & outputs_in_registered_pipeline
)

# After adding modular pipeline nodes into the graph,
# There is a chance that the graph with these nodes contains cycles if
Expand Down Expand Up @@ -456,7 +448,7 @@ def create_modular_pipelines_tree_for_registered_pipeline(
or not node.belongs_to_pipeline(registered_pipeline_id)
):
continue
if not node.modular_pipelines or node_id in root_children_ids:
if not node.modular_pipelines or node_id in root_parameters:
modular_pipelines_tree[ROOT_MODULAR_PIPELINE_ID].children.add(
ModularPipelineChild(
node_id, self.nodes.get_node_by_id(node_id).type
Expand Down
1 change: 1 addition & 0 deletions package/tests/test_api/test_rest/test_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ def assert_example_data(response_data):
"children": [
{"id": "0ecea0de", "type": "data"},
{"id": "f1f1425b", "type": "parameters"},
{"id": "f0ebef01", "type": "parameters"},
{"id": "uk", "type": "modularPipeline"},
],
"id": "__root__",
Expand Down

0 comments on commit 475a9c8

Please sign in to comment.