Skip to content

2.0 Pipelines (parametrization)#2052

Merged
jorgeorpinel merged 50 commits into
masterfrom
2.0/pipelines
Jan 12, 2021
Merged

2.0 Pipelines (parametrization)#2052
jorgeorpinel merged 50 commits into
masterfrom
2.0/pipelines

Conversation

@jorgeorpinel
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel commented Dec 23, 2020

Based on https://github.com/iterative/dvc/wiki/Parametrization

@shcheklein shcheklein temporarily deployed to dvc-landing-2-0-pipelin-enhjqc December 23, 2020 23:47 Inactive
@jorgeorpinel jorgeorpinel mentioned this pull request Dec 23, 2020
12 tasks
@jorgeorpinel jorgeorpinel changed the title 2.0 pipelines 2.0 Pipelines (parametrization) Dec 23, 2020
Comment on lines +150 to +153
## Examples: Print all parameters

Following the previous example, we can use `dvc params diff` to list all of the
param values available in the <abbr>workspace</abbr>:
Copy link
Copy Markdown
Contributor Author

@jorgeorpinel jorgeorpinel Dec 23, 2020

Choose a reason for hiding this comment

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

Note: This example has no changes (which would be out of scope) but I moved the one about Python param files to the bottom so it looks like this is added.

It's the next example the one I added.

@jorgeorpinel jorgeorpinel changed the title 2.0 Pipelines (parametrization) 2.0 Pipelines ("parametrization") Dec 24, 2020
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc December 24, 2020 04:46 Inactive
Comment thread content/docs/command-reference/params/index.md Outdated
Comment thread content/docs/user-guide/dvc-files.md Outdated
Comment thread content/docs/user-guide/dvc-files.md Outdated
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc December 27, 2020 00:42 Inactive
@jorgeorpinel jorgeorpinel changed the title 2.0 Pipelines ("parametrization") 2.0 Pipelines (ParametErization) Dec 27, 2020
@jorgeorpinel jorgeorpinel changed the title 2.0 Pipelines (ParametErization) 2.0 Pipelines (Parametrization) Dec 27, 2020
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc December 27, 2020 03:23 Inactive
Comment thread content/docs/user-guide/dvc-files.md Outdated
Comment thread content/docs/user-guide/dvc-files.md Outdated
first (to load any values from params files listed in `vars`).
- `foreach` is also incompatible with local `vars` at the moment.

The substitution expression supports these forms:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just to check, how correct the substitution expression term is?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it also feels that we should define this way before we jumped into different options for vars

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how correct the substitution expression term is?

Will research ⌛

should define this way before

We do intro the term early on before using ${} in any example. Do you mean putting "substitution expression supports these forms..." before dvc.yaml examples?

Copy link
Copy Markdown
Contributor Author

@jorgeorpinel jorgeorpinel Jan 14, 2021

Choose a reason for hiding this comment

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

OK @shcheklein, on the question of the "substitution expression" I here some the alternatives:

  • We call it "${} expression" and use "interpolation" to explain it in the original wiki page, but that latter term seemed too mathematical to me.
  • Helm (K8s YAML) calls it a "directive" (e.g. {{ .Something }}) and uses words "value", "generated", and "render/inject/insert" for the explanations.

    BTW apparently there are some strong opinions ([1], [2]) out there against templating YAML 😆

  • Kustomize (YAML) uses a different approach (template-free) with terms like "generate", "variants" and "overlays" — not directly comparable but listing here because it's an interesting alternative, maybe we can consider adopting something like that in the future.

    See also yq

  • Handlebars (HTML/JS) calls them "handlebars expressions" (e.g. {{prop}}); uses "generate/render", "property/value", "replace"
  • Twig/Django (HTML/PHP): "tags/filters" (e.g. {% var %}, {{ function }}); "render", "generate", "values"
  • Mustache (Ruby, JS, etc.): "tags" (e.g. {{key}}); "render", "replace", "values".
  • Go (text): "actions" (e.g. {{ "string" }}); "values", "evaluation", "generate"
  • Bash calls $(command) a "command substitution"

Conclusions:

  • "Substitution" is not very used out there, I guess the $ in our syntax reminded me more of Bash than of modern templating engines.
  • For ${}, just "expression" is OK. Most templating formats have a special term though, usually "tag" for value injections.
  • For related terms, I think in our case "replace" and "generate" are the most appropriate.

Comment thread content/docs/user-guide/dvc-files.md Outdated
Comment thread content/docs/user-guide/dvc-files.md Outdated
> To use the expression literally in `dvc.yaml`, escape it with a backslash,
> e.g. `\${...`.

### Defining multiple stages at once
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about foreach/do - using loops to generate stages

Copy link
Copy Markdown
Contributor Author

@jorgeorpinel jorgeorpinel Jan 8, 2021

Choose a reason for hiding this comment

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

I'm not sure we want to introduce the concept of "loops" (even when its implied by the foreach field name); According to treeverse/dvc#5181 (comment), the items given to foreach are not considered ordered so it doesn't seem like a normal loop to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(This title is Generating multiple stages now BTW.)

Comment thread content/docs/user-guide/dvc-files.md Outdated
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 00:18 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 00:22 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 04:26 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 04:28 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 04:29 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 04:33 Inactive
jorgeorpinel added a commit that referenced this pull request Jan 8, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 04:38 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 04:38 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-2-0-pipelin-enhjqc January 8, 2021 06:02 Inactive
@shcheklein
Copy link
Copy Markdown
Contributor

Okay, it's impossible to iterate on this PR further (GH breaks at this stage, it's impossible to follow all the comments, etc). Let's merge and work separately on the next version. The biggest concern is still structure:

Screen Shot 2021-01-08 at 1 58 52 PM

All these titles do not look good to me. They are fine for Basic Concepts I guess, but not as a top level section in the User Guide. They should higher level.

After that we define the expected structure we can start iterating on the actual content.

  • advanced-dvc.yaml - is too heavy and a mix of a reference (precise spec) and guide (titles are high level, no structure - story like)
  • do we need two seprate dvc.yaml sections? why do we consider basic templating and advanced feature?
  • .dvc file - is clearly a reference thing. Where do we put it?

@skshetry

This comment has been minimized.

@shcheklein
Copy link
Copy Markdown
Contributor

shcheklein commented Jan 9, 2021

@skshetry I agree that structure is not ideal, we'll iterate on it next thing. Let's merge it and do it in the next PR, this is one is not manageable.

Re the split/non-split - to the large extent if feels that it's a matter of preferences and we won't find a solution that fits all. I think the current approach is to keep pages reasonably short, to a single point (e.g. it's bad to have .dvc and the whole dvc.yaml - they are too different concepts. Can dvc.yaml fit a single page - good question-let's see in the next PR. I really hope so, I'm though worried that about some points I describe below, e.g. complexity of the right hand bar).

Usual reasons to split (besides personal preferences, which should not serve as an argument unless we do a focus group testing):

  • have a clear link to separate page that describes one thing (good for SEO, since term becomes part of the URL, page has tailored meta section, etc)
  • when you have too much on a single page- you won't have any navigation at all (due to multiple levels of nesting). Or we'll have to complicate right hand side bar.

Why is it neglected so much?

Nothing is neglected (e.g. we changed Get started when we had time from 10+ pages to 4). But I find traversing through those pages difficult is not an argument that we can use as a guideline to my mind. If you have some good structure or guideline in mind, please suggest it.

@jorgeorpinel
Copy link
Copy Markdown
Contributor Author

Merged and moved discussion to #2059 (comment).

@jorgeorpinel
Copy link
Copy Markdown
Contributor Author

It feels like we are splitting pages arbitrarily (just look at installation instructions).

@skshetry I don't see how Install pages are spit arbitrarily. But if you'd like to elaborate or have better examples feel free to open a separate issue. Thanks

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