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

cmd ref: review root ref to be aligned with other commands #404

Closed
4 tasks done
shcheklein opened this issue Jun 3, 2019 · 13 comments · Fixed by #408
Closed
4 tasks done

cmd ref: review root ref to be aligned with other commands #404

shcheklein opened this issue Jun 3, 2019 · 13 comments · Fixed by #408
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: ref Content of /doc/*-reference good first issue Good for newcomers type: enhancement Something is not clear, small updates, improvement suggestions

Comments

@shcheklein
Copy link
Member

shcheklein commented Jun 3, 2019

See https://dvc.org/doc/commands-reference/root.

i.

  • The layout of this command reference is outdated.

  • It should be the same as for the cache for example.

ii.

  • Description should include a better motivation for this command - mostly ease of generating stage files no matter where you run the dvc run.

  • Examples should be actionable and meaningful. It's should be easy to run commands to see the results.

iii.

@shcheklein shcheklein added good first issue Good for newcomers command-reference type: enhancement Something is not clear, small updates, improvement suggestions labels Jun 3, 2019
@vibhor98

This comment has been minimized.

shcheklein added a commit that referenced this issue Jun 11, 2019
Fix #404: Refactor root and improve its style.
@shcheklein shcheklein reopened this Jun 11, 2019
@shcheklein
Copy link
Member Author

Partially done by @vibhor98 . I've updated the description a little considered the changes done.

@jorgeorpinel jorgeorpinel changed the title root: refactor and improve style to be aligned with other commands cmd ref: revise root ref to be aligned with other commands Jan 19, 2020
@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) and removed command-reference labels Jan 19, 2020
@jeremydesroches
Copy link
Contributor

@jorgeorpinel I’ll work on closing this out.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 30, 2020

@shcheklein the first bullet states "motivation for this command - mostly ease of generating stage files no matter where you use dvc run". Actually, we no longer see run as the main and recommendad way to generate dvc.yaml files, so is that motivation still valid? I think almost. Maybe something more like "ease of generating stages in dvc.yaml files programmatically or with the dvc run helper command, regardless of where it's used."

Cc @jeremydesroches - does this make sense to you? You may want to review https://dvc.org/doc/user-guide/dvc-files-and-directories and https://dvc.org/doc/command-reference/run 🙂

P/s feel free to contact me with questions or even faster at out Discord channel dvc.org/chat

@jeremydesroches
Copy link
Contributor

@jorgeorpinel Yes, that does make sense to me. The examples work with dvc run, but don't seem as useful as direct or programmatic usage in dvc.yaml files. If the recommended use and primary motivation is to use it in dvc.yaml files then the examples could be updated too.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 30, 2020

Yes, that does make sense to me

Great! Please use the motivation I suggested as the guiding idea for finishing updating the command's description, Jeremy.

then the examples could be updated too.

I agree, but let's keep the current PR small for now, and just update the existing content or remove examples if they're no longer relevant? Leaving it for another PR to add new ones (so please don't use the special word "Fixes" in the PR description for now). Thanks

ALTHOUGH by programmatically we basically mean shell scripts here I think (since dvc root is a command)... And I'm not sure people will generate dvc.yaml files with shell scripts very often. I guess you can invoke the command from any programming language to... But for Python specifically there's probably a better way using the dvc module.

So maybe let's wait to see what @shcheklein says. Let me also ping @efiop (hi) — what are the use cases of dvc root in 1.x? Thanks! UPDATE: Never mind, sorry Ruslan.

@jeremydesroches
Copy link
Contributor

OK, I'll wait for comments before updating motivation and examples. I can see how it might be used in a script to "template" an experiment or stage to be used in different projects, but that type of example might be too detailed for a command reference.

Removed "Fixes" from the PR description.

@jorgeorpinel

This comment has been minimized.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 30, 2020

Sorry I let you start working on this without thinking it through. It's just a pretty old issue... But anyway!

OK, I had a discussion with myself (and found a previous conversations in #408) and yes, it's main use cases seem to be 1) as a way to get the project root from shell scripts that generate dvc.yaml files either with the dvc run helper or programmatically in some other way; and 2) to embed it directly in dvc run (only POSIX terminals).

So the current Examples are OK topic-wise (just need to be "actionable and meaningful" as described above), but another one with an actual shell script (for Linux) would be nice (not required for your current PR @jeremydesroches).

direct or programmatic usage in dvc.yaml files

BTW this isn't possible, dvc.yaml doesn't accept any sort of variables, it's just YAML.

@iterative iterative deleted a comment from jeremydesroches Jul 30, 2020
@jeremydesroches
Copy link
Contributor

jeremydesroches commented Jul 30, 2020

No problem at all. I was hoping to work on an issue that required a bit of investigation and discussion. I learned a lot, so mission accomplished!

So based on your discussion (haha) it sounds like the the current PR won't need much more adjustment. I'll find a newer issue to work on next. (Please let me know if any suggestions or preferences.)

BTW this isn't possible, dvc.yaml doesn't accept any sort of variables, it's just YAML.

I see, so you couldn't have a dvc.yaml file with a $(dvc root) variable — but you could use $(dvc root) in the script to generate a dvc.yaml file with references to the root directory, relative to the working directory where the script is run. Right?

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 30, 2020

Yep, just to address the couple reviews already open in #1637 should wrap it up.

Right?

Correct. Once generated, they would be hard coded in dvc.yaml (or in a .dvc file for that matter, since you could do something like dvc add $(dvc root)/data/words.txt. See https://dvc.org/doc/command-reference/add if you haven't 🙂

@jorgeorpinel
Copy link
Contributor

OK, #1637 almost closed this (thanks @jeremydesroches) but we still need that shell script example. I updated the description of this issue (who knew it would be so hard to close...)

@jorgeorpinel jorgeorpinel changed the title cmd ref: revise root ref to be aligned with other commands cmd ref: review root ref to be aligned with other commands Jan 6, 2021
@jorgeorpinel
Copy link
Contributor

This is mostly done. Moved the remaining part to #2076. Closing!

@iesahin iesahin added the C: ref Content of /doc/*-reference label Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: ref Content of /doc/*-reference good first issue Good for newcomers type: enhancement Something is not clear, small updates, improvement suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants