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

repro: enforce targets (and update desc) when -R is present? [qa] #4292

Closed
jorgeorpinel opened this issue Jul 27, 2020 · 5 comments
Closed
Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC ui user interface / interaction

Comments

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 27, 2020

The -R option of dvc repro has no effect when no targets that are directories are also given. For example, dvc repro -R is the same as just dvc repro. For UI and consistency improvement I propose

    • targets are enforced when -R is used, or at least WARN msg is printed otherwise so the user is aware he might be expecting something that's not being done.
    • Even if targets are given, at least one of them should be an existing directory path, or the above enforcement/warning will happen.
  1. (This may already be the case) if the target directory has no .dvc files or dvc.yaml, print a separate warning about that?

UPDATE: See #4292 (comment) below for an alternative solution — maybe easier.


THE REMAINING TEXT HAS BEEN EXTRACTED TO #4393 (so stop reading)

Separately (but related), the targets param accepted by dvc repro is described (in its help output) as Stages to reproduce. 'dvc.yaml' by default. This message is confusing as dvc.yaml is not a stage. In docs we're using Stage or .dvc file to reproduce but that's not correct either because apparently paths to dvc.yaml files are also accepted.

  • Should we use Stage or path to dvc.yaml or .dvc file to reproduce instead?
  • ^ still doesn't cover the case of accepting directory paths with -R, but that we can explain in that argument's description:
-  -R, --recursive       Reproduce all stages in the specified directory.
+  -R, --recursive       Reproduce all stages in the specified directories.
+                        These can be given as `targets` arguments.

and I'll explain this special case further in the cmd ref. Options.

@jorgeorpinel jorgeorpinel added enhancement Enhances DVC ui user interface / interaction labels Jul 27, 2020
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel
Copy link
Contributor Author

  1. For UI and consistency improvement I propose...

An alternative to this, as I discussed with @efiop, is to make -R accept an argument, instead of relying on targets. His answer was that it's still basically a target so why not just use the command's argument(s), which was convincing. However, I just noticed that the -c option does accept a <path> argument (specifically for directories), so maybe after all (for consistency) the best option is to add an arg to -R, instead of points 1-3 above (and apply the logic to that arg).

@jorgeorpinel jorgeorpinel added the discussion requires active participation to reach a conclusion label Jul 28, 2020
@jorgeorpinel jorgeorpinel changed the title repro: enforce targets (and update desc?) when -R is present repro: enforce targets (and update desc?) when -R is present? Jul 28, 2020
@jorgeorpinel jorgeorpinel changed the title repro: enforce targets (and update desc?) when -R is present? repro: enforce targets (and update desc?) when -R is present? [qa] Jul 28, 2020
@efiop
Copy link
Contributor

efiop commented Jul 31, 2020

However, I just noticed that the -c option does accept a argument (specifically for directories), so maybe after all (for consistency) the best option is to add an arg to -R, instead of points 1-3 above (and apply the logic to that arg).

-c is a whole different thing, it is in no way related to targets. It just makes dvc chdir to that directory and work from there.

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jul 31, 2020

True, you could combine the args like dvc repo -c some/path stage1 stage2. OK, never mind about the alternative then.

P/s I wonder what happens if you supply a path to a dvc.yaml file as target along with a -c, is it made relative to the -c path arg?

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Jul 31, 2020

P/s I wonder what happens if you supply a path to a dvc.yaml file as target along with a -c, is it made relative to the -c arg?

Yes, it is. So, strangely, dvc repro = dvc repro -c subdir/ ../dvc.yaml 🤷

UPDATE: Added quick note about this in iterative/dvc.org@d30bc63.

jorgeorpinel added a commit to iterative/dvc.org that referenced this issue Jul 31, 2020
shcheklein pushed a commit to iterative/dvc.org that referenced this issue Aug 5, 2020
* cmd: review repro examples
per #1572 (comment)

* cmd: fic tyupo in get-url

* cmd: updates to repro
per #1572 (review)

* cmd: rewrite repro -P desc
rel #1572 (review)

* cmd: simplified and generalize repro targets desc and DVC file mention
per #1572 (comment)
and #1572 (comment)

* cmd: minor update for repro desc wording
per #1572 (comment)

* term: don't use "synchronize" in the context of checkout

* cmd: rewrite Downstream example and added info for sequential execution of stages

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* cmd: Updated Downstream example

* Update content/docs/command-reference/repro.md

* repro: Updated Downstream example

* Update content/docs/command-reference/repro.md

* cmd: updated last para for the description of --downstream and improved formatting

* cmd: review language of init --subdir

* term: revuew usage of "granular", esp. around init --subdir

* repro.md: updated Downstream example

* cmd: improve init --subdir explanation

* cmd: add info about nested subrepos to init

* cmd: fix -P option desc.
per #1615 (review)

* cmd: improve explanation on how --subdir affects commands
per #1615 (review)

* cmd: simplify nested structures explanation in init
per #1615 (comment)

* guide: add note aboud `cp` not being a download in external deps
per #1643 (review)

* cmd: add note about what --cwd means to repro
per iterative/dvc#4292 (comment)

* guide: nvmd! removing that note in external deps
per 42b670f#r41087633

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* cmd: more small updates to init

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Restyled by prettier

* cmd: rewrap metrics diff usage paragraph

* term: remove "just" from -j desc in 3 refs
per eda27fc

* cmd: add command examples to init --subdir use cases
per #1615

* cmd: explain nested repo and projects of all kinds outside of --subdir
per #1615 (review)

* cmd: remove bold names to nested and not-nested structure examples in init --subdir
per #1615 (review)

* cmd: standardize --jobs option in all refs
per #1615 (review)
et al.

* cmd: add speed note to --jobs desc in all refs.
per #1615 (review)

* cmd: change versioning command example in init
per #1615 (review)

* cd: change repo comments in init --subdir examples
for #1615 (review)

* cmd: improve note on DVC submodules a little
for #1615 (review)

* cmd: better explain why isolation is important in --subdir bullet
per #1615 (review)

* cmd: split last --subdir cases explicitly as 2 bullets
per #1615 (comment)

* cmd: remove most notes and code block examples about nesting projects/repos in init
per #1615 (review)

Co-authored-by: sarthakforwet <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
shcheklein pushed a commit to iterative/dvc.org that referenced this issue Aug 5, 2020
* cmd: review repro examples
per #1572 (comment)

* cmd: fic tyupo in get-url

* cmd: updates to repro
per #1572 (review)

* cmd: rewrite repro -P desc
rel #1572 (review)

* cmd: simplified and generalize repro targets desc and DVC file mention
per #1572 (comment)
and #1572 (comment)

* cmd: minor update for repro desc wording
per #1572 (comment)

* term: don't use "synchronize" in the context of checkout

* cmd: rewrite Downstream example and added info for sequential execution of stages

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* cmd: Updated Downstream example

* Update content/docs/command-reference/repro.md

* repro: Updated Downstream example

* Update content/docs/command-reference/repro.md

* cmd: updated last para for the description of --downstream and improved formatting

* cmd: review language of init --subdir

* term: revuew usage of "granular", esp. around init --subdir

* repro.md: updated Downstream example

* cmd: improve init --subdir explanation

* cmd: add info about nested subrepos to init

* cmd: fix -P option desc.
per #1615 (review)

* cmd: improve explanation on how --subdir affects commands
per #1615 (review)

* cmd: simplify nested structures explanation in init
per #1615 (comment)

* guide: add note aboud `cp` not being a download in external deps
per #1643 (review)

* cmd: add note about what --cwd means to repro
per iterative/dvc#4292 (comment)

* guide: nvmd! removing that note in external deps
per 42b670f#r41087633

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* cmd: more small updates to init

* Update content/docs/command-reference/repro.md

* Update content/docs/command-reference/repro.md

* Restyled by prettier

* cmd: rewrap metrics diff usage paragraph

* term: remove "just" from -j desc in 3 refs
per eda27fc

* cmd: add command examples to init --subdir use cases
per #1615

* cmd: explain nested repo and projects of all kinds outside of --subdir
per #1615 (review)

* cmd: remove bold names to nested and not-nested structure examples in init --subdir
per #1615 (review)

* cmd: standardize --jobs option in all refs
per #1615 (review)
et al.

* cmd: add speed note to --jobs desc in all refs.
per #1615 (review)

* cmd: change versioning command example in init
per #1615 (review)

* cd: change repo comments in init --subdir examples
for #1615 (review)

* cmd: improve note on DVC submodules a little
for #1615 (review)

* cmd: better explain why isolation is important in --subdir bullet
per #1615 (review)

* cmd: split last --subdir cases explicitly as 2 bullets
per #1615 (comment)

* cmd: remove most notes and code block examples about nesting projects/repos in init
per #1615 (review)

* cmd: add basic text about nesting to init

* cmd: revert nested block examples back into --subdir
per #1661 (review)

* cmd: remove new section about nesting to revert more

Co-authored-by: sarthakforwet <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
@jorgeorpinel jorgeorpinel changed the title repro: enforce targets (and update desc?) when -R is present? [qa] repro: enforce targets (and update desc) when -R is present? [qa] Aug 15, 2020
@mattseddon mattseddon closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC ui user interface / interaction
Projects
None yet
Development

No branches or pull requests

3 participants