Skip to content
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

feature/#508 change Pipeline and PipelineConfig documentation #535

Conversation

toan-quach
Copy link
Member

@toan-quach toan-quach commented Aug 28, 2023

#508

Note: This is a series of changes to taipy-doc regarding the removal of PipelineConfig, Model and Repositories topic. I will break it down so section level (concept, config, entities) and create a PR for each section to keep the changes minimal and relevant so as not to block any restructuring work being done by others.

@toan-quach toan-quach marked this pull request as ready for review August 29, 2023 07:18
Copy link
Member

@FabienLelaquais FabienLelaquais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes propsed.

docs/manuals/core/concepts/execution-flow.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/scenario.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/scenario.md Outdated Show resolved Hide resolved
data nodes together. It can also be broken down to smaller graphs for execution by defining a `Sequence^`.
A sequence is a subset of tasks derives from the scenario's set of tasks, forming a smaller executable DAG that
can be submitted separately from the scenario DAG. A scenario can also contain a set of additional data nodes
outside of the scenario DAG to represent additional data related to the scenario but are not executable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the need for the "but are not executable" fragment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FabienLelaquais The additional DNs are for data that are related to the scenario but are not part of the DAG, so they are not submittable. Hence not executable 😃

Copy link
Member

@FabienLelaquais FabienLelaquais Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course they are not. That's my point. Why even mention it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I wanted to make it clear that it doesn't belong to the DAG as we did mention before that, a DAG is a collection of tasks and datanodes. So I'm afraid user might be confused if they should provide the data nodes again aside from the task for the DAG

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is that indeed, tasks are executable. In a sense, sequences and scenarios are as well.
But unless you understood nothing, data nodes never will be.

The addition of "[data nodes] [...] are not executable" really seems strange here. Is similar to saying: "Oh by the way, giraffes cannot talk.".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see what you meant, then how about "A scenario can also contain a set of additional data nodes that are not part of the scenario DAG to represent additional data related to the scenario."?

docs/manuals/core/concepts/scope.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/sequence.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/sequence.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/task.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/task.md Outdated Show resolved Hide resolved
docs/manuals/core/config/pipeline-config.md Show resolved Hide resolved
docs/manuals/core/concepts/cycle.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/index.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/index.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/index.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/job.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/scope.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/scope.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/sequence.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/sequence.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/sequence.md Outdated Show resolved Hide resolved
Copy link
Member

@FabienLelaquais FabienLelaquais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

docs/manuals/core/concepts/execution-flow.md Outdated Show resolved Hide resolved
data nodes together. It can also be broken down to smaller graphs for execution by defining a `Sequence^`.
A sequence is a subset of tasks derives from the scenario's set of tasks, forming a smaller executable DAG that
can be submitted separately from the scenario DAG. A scenario can also contain a set of additional data nodes
outside of the scenario DAG to represent additional data related to the scenario but are not executable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is that indeed, tasks are executable. In a sense, sequences and scenarios are as well.
But unless you understood nothing, data nodes never will be.

The addition of "[data nodes] [...] are not executable" really seems strange here. Is similar to saying: "Oh by the way, giraffes cannot talk.".

docs/manuals/core/concepts/scenario.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/scenario.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/scenario.md Outdated Show resolved Hide resolved
docs/manuals/core/concepts/sequence.md Show resolved Hide resolved
@toan-quach toan-quach merged commit 0d73957 into develop Aug 30, 2023
1 check passed
@toan-quach toan-quach deleted the feature/#508-change-pipeline-and-pipeline-config-documentation branch August 30, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants