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.
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
Revise modular pipelines docs #3948
Revise modular pipelines docs #3948
Changes from 30 commits
18a46ee
8a7d370
9fbf6b3
653dcde
53f94fe
8678eb7
23b0989
682efb1
c8e75a1
9ba96fc
7c93b58
bdae1ec
3d37b5c
c907c28
5999491
e235a24
7b06d9c
952c4fa
8fcc3c4
a58d31e
77ed3aa
4e2c8ac
faa08b2
84f444f
aa4a3c2
0ac37be
2fbf15e
0c14ce2
6b36ab9
bc04d86
c46dab3
1cfc4d7
cf63f67
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 55 in docs/source/faq/faq.md
GitHub Actions / vale
[vale] docs/source/faq/faq.md#L55
Raw output
Check warning on line 56 in docs/source/faq/faq.md
GitHub Actions / vale
[vale] docs/source/faq/faq.md#L56
Raw output
Check warning on line 56 in docs/source/faq/faq.md
GitHub Actions / vale
[vale] docs/source/faq/faq.md#L56
Raw output
Large diffs are not rendered by default.
Check notice on line 5 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L5
Raw output
Check warning on line 5 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L5
Raw output
Check warning on line 17 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L17
Raw output
Check notice on line 17 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L17
Raw output
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.
And similarly in other nodes, because in all the places this
name
is used, it is clear that it's anode
.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 would prefer to leave this as it is because it's currently how it's written in our starters, and I don't want to confuse users with different naming since our example is based on that starter.
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.
Why is it not using
namespace
? Isn't this page supposed to be about that?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.
In my opinion, the point of the first section titled "How to reuse your pipelines" is to show that you can reuse your pipelines without needing namespaces. However, by adding namespaces, you can reuse your pipelines multiple times, with namespaces providing the necessary support. So my point is: reuse is the primary idea, and namespaces are an enhancement.
Check warning on line 56 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L56
Raw output
Check warning on line 71 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L71
Raw output
Check warning on line 71 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L71
Raw output
Check warning on line 71 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L71
Raw output
Check warning on line 71 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L71
Raw output
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 find this confusing because in the code snippet above :
base_data_science
is created insidecreate_pipeline()
...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.
Other than in the explanation there's no actual reference to
data_science
anywhere in the code examples on this page.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 fixed it by adding the comment
# data_science pipeline creation function
before thecreate_pipeline()
function. However, as I understand it, I have to usecreate_pipeline()
name to ensure the pipeline will be autodiscovered. Do you have other ideas on how to highlight thedata_science
name inside the code?Check warning on line 73 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L73
Raw output
Check warning on line 73 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L73
Raw output
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.
Ok this is very very unclear. Why to reuse an existing pipeline that's designed to be modular you're running
kedro pipeline create
? Isn't a more intuitive flow to define this abstract, unregistered pipeline once and then initialize it's actual instances to be registed within the sameproject/pipelines/pipeline
folder?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.
Thank you for that proposal, @yury-fedotov. When I wrote that manual, I also thought about how I can reuse a pipeline created with
kedro pipeline create
command since the pipeline object is created inside the function. Should I create an additional pipeline with thekedro pipeline create
command to reuse the first one? I believe it's important to provide our users with best practices, and I would be happy to hear more opinions on that. Maybe @astrojuanlu and @merelcht have some insights.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 my point of view it doesn't matter how you create the pipelines, what's important is how you re-use them in the end. It's up to the user if they want the pipelines to be in the same folder or separated out (which is what happens when you do
kedro pipeline create
).Check warning on line 87 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L87
Raw output
Check warning on line 103 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L103
Raw output
Check notice on line 117 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L117
Raw output
Check warning on line 117 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L117
Raw output
Check warning on line 117 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L117
Raw output
Check warning on line 117 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L117
Raw output
Check warning on line 125 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L125
Raw output
Check warning on line 133 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L133
Raw output
Check warning on line 135 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L135
Raw output
Check warning on line 139 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L139
Raw output
Check warning on line 146 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L146
Raw output
Check warning on line 152 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L152
Raw output
Check warning on line 152 in docs/source/nodes_and_pipelines/namespaces.md
GitHub Actions / vale
[vale] docs/source/nodes_and_pipelines/namespaces.md#L152
Raw output