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

Misc. updates #1881

Merged
merged 13 commits into from
Oct 23, 2020
Merged

Misc. updates #1881

merged 13 commits into from
Oct 23, 2020

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Oct 20, 2020

@shcheklein shcheklein temporarily deployed to dvc-landing-jorge-5buputd875fy October 20, 2020 17:24 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: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
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
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 iterative/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
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 iterative/dvc#4761 to address this

Copy link
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
Member

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