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

Adding a note on dvc plots diff --targets with revisions usage #1549

Merged
merged 3 commits into from
Jul 10, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions content/docs/command-reference/plots/diff.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,21 @@ please see `dvc plots`.

- `--targets <path>` - specific metric files to visualize. These must be listed
in a [`dvc.yaml`](/doc/user-guide/dvc-files-and-directories#dvcyaml-file) file
(see the `--plots` option of `dvc run`).
(see the `--plots` option of `dvc run`). When you are specifying multiple
arguments for `--targets` and `revisions`, use `--` after all the arguments
for `--targets`:
Comment on lines +50 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(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.:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll incorporate these changes in a new PR

Copy link
Contributor

Choose a reason for hiding this comment

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

I see it's done in #1567, thanks.


```dvc
$ dvc plots diff --targets t1.json t2.csv -- HEAD v1 v2
```

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:
Comment on lines +58 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Just

Suggested change
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:

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder is this should be the first recommendation though, not --. I also wonder which other commands can benefit from the -- syntax.

Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT @imhardikj ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think order is correct. As mentioned in #1544 (comment) it's almost natural instinct to give targets first and then revisions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

-- 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.


```dvc
$ dvc plots diff HEAD v1 v2 --targets t1.json t2.csv
```

- `-o <path>, --out <path>` - name of the generated file. By default, the output
file name is equal to the input filename with a `.html` file extension (or
Expand Down Expand Up @@ -111,15 +125,6 @@ file:///Users/usr/src/plots/logs.csv.html

![](/img/plots_diff.svg)

> Alternatively, you can also run above statement as:
>
> ```dvc
> $ dvc plots diff --targets logs.csv -- HEAD 0135527
> ```
>
> When you're specifying multiple revisions after `--targets`, use `--` so that
> argument parser doesn't confuse them as options for `--targets` argument.

## Example: Confusion matrix

We'll use tabular metrics file `classes.csv` for this example:
Expand Down