Adding a note on dvc plots diff --targets with revisions usage #1549
Conversation
| > Alternatively, you can also run above statement as: | ||
| > Please note that when you're specifying `--targets` options with multiple | ||
| > `revisions`: | ||
| > | ||
| > ```dvc | ||
| > $ dvc plots diff --targets t1.json t2.csv HEAD v1 v2 | ||
| > ``` | ||
| > | ||
| > This statement will show an error because argument parser confuses `revisions` | ||
| > as arguments for `--targets` option. To avoid this error use `--` after all | ||
| > the arguments for `--targets`: |
There was a problem hiding this comment.
Let's move this note up to the description as a regular paragraph, and just make a quick reminder in this note (block quote) here in the example.
There was a problem hiding this comment.
Actually, per #1544 (comment) please move up into the --targets option bullet text instead. Feel free to use several paragraphs and even code blocks or block quote notes in there, but do try to summarize all this a little in both places. Thanks
There was a problem hiding this comment.
please, let's try to keep it really simple. The problem itself is very simple and clear and one sentence should be enough in the description or option description to my mind.
There was a problem hiding this comment.
Yeah, by "summarize all this a little" I meant summarize it a lot too 🙂
There was a problem hiding this comment.
Continued in #1549 (review)
| (see the `--plots` option of `dvc run`). When you are specifying multiple | ||
| arguments for `--targets` and `revisions`, use `--` after all the arguments | ||
| for `--targets`: |
There was a problem hiding this comment.
| (see the `--plots` option of `dvc run`). When you are specifying multiple | |
| arguments for `--targets` and `revisions`, use `--` after all the arguments | |
| for `--targets`: | |
| (see the `--plots` option of `dvc run`). When specifying multiple targets and | |
| `revisions`, you may use `--` after this option's arguments, e.g.: |
There was a problem hiding this comment.
I'll incorporate these changes in a new PR
There was a problem hiding this comment.
I see it's done in #1567, thanks.
| Without `--`. this statement will show an error as argument parser confuses | ||
| `revisions` as arguments for `--targets` option. Alternatively, you can also | ||
| run above statement as: |
There was a problem hiding this comment.
Just
| Without `--`. this statement will show an error as argument parser confuses | |
| `revisions` as arguments for `--targets` option. Alternatively, you can also | |
| run above statement as: | |
| Alternatively, you can also run above statement as: |
There was a problem hiding this comment.
I wonder is this should be the first recommendation though, not --. I also wonder which other commands can benefit from the -- syntax.
There was a problem hiding this comment.
I think order is correct. As mentioned in #1544 (comment) it's almost natural instinct to give targets first and then revisions.
There was a problem hiding this comment.
-- will be used only when a option accepts multiple arguments and at least one positional argument. This can be used in metrics diff. But right now metrics diff doesn't have any example which shows usage of --targets option.
There was a problem hiding this comment.
But the same note applies to metrics diff? If so please copy it to that reference as well in your new PR, or a subsequent one 🙂 Adding an targets example (similar to the one here) would be a nice addition too but not necessary.
Fixes #1425
Continuation of #1544
@jorgeorpinel @pared I have included possible uses cases in the note