Skip to content

Misc. updates#1881

Merged
jorgeorpinel merged 13 commits into
masterfrom
jorge
Oct 23, 2020
Merged

Misc. updates#1881
jorgeorpinel merged 13 commits into
masterfrom
jorge

Conversation

@jorgeorpinel
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel commented Oct 20, 2020

@shcheklein shcheklein temporarily deployed to dvc-landing-jorge-5buputd875fy October 20, 2020 17:24 Inactive
Comment thread content/docs/command-reference/repro.md Outdated
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 00:03 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 00:03 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 00:08 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review October 21, 2020 00:20
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 00:21 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 00:27 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 00:33 Inactive
Comment on lines +36 to +40
This type of metrics files are created by users, or generated by user data
processing code, and get defined with the `-p` (`--plots`) and
`--plots-no-cache`) options of `dvc run`. `dvc plots` subcommands can work with
plots files committed to a Git repo history, data files controlled by DVC, or
any other file in system.
processing code, and can be defined in `dvc.yaml` stages (using the `--plots`
and `--plots-no-cache` options if using `dvc run`). `dvc plots show` and
`dvc plots diff` can work with any valid plots files in the system, whether
tracked by Git or DVC, or not.
Copy link
Copy Markdown
Contributor Author

@jorgeorpinel jorgeorpinel Oct 21, 2020

Choose a reason for hiding this comment

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

Finishes #1800 @pared @shcheklein but note that since plots modify does require that plots are defined in dvc.yaml, I couldn't change this to be as general as suggested initially (summarized in #1809 (comment)):

if we explain in the plots concept itself that plots are not only dvc.yaml based people won't expect dvc.yaml to be present in the first place.

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.

I like the shift from get defined to can be defined. This is on point what we actually did.

can work with any valid plots files in the system

While it is something we need to do (as part of treeverse/dvc#4446, currently we cannot dvc plots show {target} in no-repo case, as we will get error that '.' is not a git repo.

Copy link
Copy Markdown
Contributor

@pared pared Oct 21, 2020

Choose a reason for hiding this comment

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

Created treeverse/dvc#4761 to address this

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.

OK great, thanks for addressing that. In that case we can leave this text as-is, I think.

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.

I think we are still overcomplicating this. In this doc the most important points to say that users can output certain type of metrics (arrays, etc) into JSON, YAML or whatnot and DVC provides a bunch of commands to deal with them - visualize, compare, etc, etc. We should def mention that it is similar to metrics, but for "continuous", etc, etc

Details that certain commands require dvc.yaml to be present can be hidden into those commands. Or there should be a separate section that starts with some motivation for creating a dvc.yaml- what benefits on top of regular files people can get from using it.

Copy link
Copy Markdown
Contributor Author

@jorgeorpinel jorgeorpinel Oct 22, 2020

Choose a reason for hiding this comment

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

Well, I was following the original idea to emphasize that plots don't have to be tracked by DVC... But you're right, this was too low level for an index. I just removed a bunch of redundant info and left the basics + some refs to where the details are. Also updated show and diff a bit again. See 27519b4.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 21, 2020 17:22 Inactive
Comment thread content/docs/command-reference/fetch.md Outdated
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 22, 2020 23:01 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 22, 2020 23:07 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 22, 2020 23:34 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 22, 2020 23:41 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorge-5buputd875fy October 22, 2020 23:59 Inactive
@jorgeorpinel jorgeorpinel merged commit 1ac5f01 into master Oct 23, 2020
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.

plots: update docs

3 participants