Fix Flowchart bug with datasets within a nested modular pipeline. #1863
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -14,6 +14,28 @@ | |||
| ) | |||
|
|
|||
|
|
|||
| def _check_is_internal_input_output( | |||
| modular_pipeline_id: str, input_node_modular_pipeline: List | |||
| ) -> Optional[str]: | |||
There was a problem hiding this comment.
The definition is a bit confusing to me, can we have -
def _is_internal_node(
modular_pipeline_id: str, internal_modular_pipelines: List
) -> Optional[str]:
|
Thanks for addressing this issue! Hope the example I provided helped localize it. I think it's a great enabler for kedro viz adoption in large projects. |
|
@yuryfedotov-mck - We've started addressing the issue you raised, but unfortunately, the solution we implemented doesn't work with deeply nested pipelines. We are doing further investigation to find a fix. Since modular pipelines are quite complex, this may take some time. Please bear with us as we work through it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #1814
Development notes
There was a bug in the logic of modular pipelines. Essentially, datasets that served as inputs and outputs to nested modular pipelines (internal_inputs/internal_outputs to modular pipeline) were mistakenly treated as external_inputs/external_outputs to the modular pipeline. This occurred because we were only checking if datasets were internal by comparing them against the nested modular pipeline, neglecting to verify against the parent modular pipeline. Now, we check against both the nested modular pipeline and the parent modular pipeline to determine whether the dataset is either an internal input/output to the modular popular or external input/output.
QA notes
You can verify that the issue is resolved by comparing it to the pipeline example shared in issue #1814.
Additionally, if you examine example number 2 in issue #1651, which includes a nested pipeline, you'll notice that the problem with 'main_pipeline.dataset_1' is fixed. Now, it's hidden inside the 'main_pipeline' node when viewed in collapsed mode.
Checklist
RELEASE.mdfile