Skip to content

plots: replace --show-json with --show-vega#3891

Merged
efiop merged 1 commit into
treeverse:masterfrom
efiop:fix-3890
May 28, 2020
Merged

plots: replace --show-json with --show-vega#3891
efiop merged 1 commit into
treeverse:masterfrom
efiop:fix-3890

Conversation

@efiop
Copy link
Copy Markdown
Contributor

@efiop efiop commented May 28, 2020 •
edited by jorgeorpinel
Loading

Fixes #3890

  • ❗ I have followed the Contributing to DVC checklist.

  • 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. If the CLI API is changed, I have updated tab completion scripts.

  • ❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

dmpetrov
dmpetrov approved these changes May 28, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmpetrov dmpetrov left a comment

Choose a reason for hiding this comment

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

tested with vega
👍

@efiop efiop merged commit 2d8c604 into treeverse:master May 28, 2020
@efiop efiop deleted the fix-3890 branch May 28, 2020 11:42
@jorgeorpinel
Copy link
Copy Markdown
Contributor

jorgeorpinel commented Jun 3, 2020

Are docs updated in an open PR somewhere? Or should I change it in one of mine?

jorgeorpinel
jorgeorpinel reviewed Jun 3, 2020
View reviewed changes
Comment thread dvc/command/plots.py
action="store_true",
default=False,
help="Show output in JSON format.",
help="Show output in VEGA format.",
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Jun 3, 2020

Choose a reason for hiding this comment

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

Suggested change
help="Show output in VEGA format.",
help="Show output in Vega format.",

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Jun 3, 2020

Choose a reason for hiding this comment

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

Same in line 216

Copy link
Copy Markdown
Contributor Author

@efiop efiop Jun 3, 2020

Choose a reason for hiding this comment

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

Actually, these are fixed. Probably did that after.

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Jun 3, 2020

Choose a reason for hiding this comment

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

I'm seeing this in the output of dvc plots show -h in 1.0.0a7+12172f which I think is the latest master version.

Copy link
Copy Markdown
Contributor Author

@efiop efiop Jun 3, 2020

Choose a reason for hiding this comment

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

@jorgeorpinel Oh, sorry, I'm blind. I looked at the bottom part that is the patch itself and not your suggestion. Will fix, thank you!

jorgeorpinel added a commit to treeverse/dvc.org that referenced this pull request Jun 3, 2020
@jorgeorpinel
cmd ref: plots --show-json -> --show-vega
5108e14 
per treeverse/dvc#3891 (comment)
@jorgeorpinel
Copy link
Copy Markdown
Contributor

jorgeorpinel commented Jun 3, 2020

Are docs updated in an open PR somewhere? Or should I change it in one of mine?

I did this in treeverse/dvc.org@5108e14 but not sure it's enough in terms of explaining the change in behavior or maybe changed outputs from existing examples.

@efiop
Copy link
Copy Markdown
Contributor Author

efiop commented Jun 3, 2020

Thank you @jorgeorpinel !

And I also forgot to update completion scripts 🤦‍♂️ Will do.

@efiop efiop mentioned this pull request Jun 3, 2020
Closed
shcheklein pushed a commit to treeverse/dvc.org that referenced this pull request Jun 11, 2020
@jorgeorpinel @dmpetrov
Regular updates & plots 1.0 update (#1382)
37f4e90 
* cmd ref: add note that move creates dirs

* cmd ref: improve structure of add ref desc.

* grammar: add some commas

* term: checksum -> hash value in dvcignore guide

* style: lower case bullet text

* cmd ref: remove some redundancy in metrics index

* cmd ref: update plots refs synopsis and descriptions
per treeverse/dvc/issues/3924 et al.

* Add plots modify cmd

* typo: CSV->csv

* term: working tree -> workspace
per treeverse/dvc/pull/3914

* cmd ref: couple improvements to add ref
per #1382 (review)
and #1382 (review)

* Update config/prismjs/dvc-commands.js

* cmd ref: update plots modify description

* cmd ref: add plots modify to nav, with a few more improvements

* cmd ref: plots --show-json -> --show-vega
per treeverse/dvc#3891 (comment)

* rename x-lab to x-label

* cmd ref: review descriptions of plots index, show, and diff

* cmd ref: review and update old plots cmds options
per treeverse/dvc#3948 et al.

* cmd ref: a couple more option updates
per #1382 (review)

* cmd ref: emphasize add works with any large file/dir
per #1382 (review)

* cmd ref: updae plots modify top half (definition, description)
per #1382 (review) al.

* cmd ref: improve all plot cmd option descriptions

* Update content/docs/command-reference/plots/modify.md

* cmd ref: review examples (mainly images) in plots modify
per #1382 (comment) et al.

* cmd ref: rephrase info about how data arrays are injected to plot templates
per #1382 (review)

* cmd ref: update info on how targets for for plots show/diff
per #1382 (review)

* cmd ref: double check all plots examples
per #1382 (comment)

* cmd ref: remove info about plots show --select

* cmd ref: update add desc
per #1382 (review)

* cmd ref: re-explain dvc add for dirs
per #1382 (review)

* cmd ref: improve description about targets in plots diff
per #1382 (review)

* cmd ref: make emoji note in plots index
per #1382 (review)

* cmd ref: remove ineffective CSV code block highlighting from plots refs
per #1382 (review)

* get started: improve intro in index

* glossary: remove external deps entry (no need)

* cmd ref: add info about column indexing for headerless tables
per #1382 (comment)

* cmd ref: update template metavar for plots subcommands
per #1382 (review)

* cmd ref: mention YAML is supported for plots
per #1382 (comment)

* cmd ref: rename template metavar again in plots
per #1382 (comment)

* cmd ref: clarify plots modify --no-csv-header
per #1382 (review)

* cmd ref: add note about plots modify in show and diff

* cmd ref: update all plots options again

* cmd ref: more updates to plots et al. per Ivan's review

* cmd ref: multiple plots diff --targets allowed

* cmd ref: update note about detault metrics in index
per #1382 (review)

* cmd ref: emphasize add --recursive is rarely needed
per #1382 (review)

* cmd ref: plots diff: update revisions arg desc
per #1382 (review)

* cmd ref: mention column names and numbers in plots {cmd} -x and -y
per #1382 (review)

* cmd ref: emphasize that metrics diff is not a real diff
per #1382 (review)

* cmd ref: simplify note on plots targets
per #1382 (review)

* cmd ref: how to id colmns in plots modify --no-csv-header
per #1382 (review)

* cmd ref: add default target behavior to plots show and diff
rel: #1382 (review)

* cmd ref: rename plots option --no-header
per treeverse/dvc/pull/4001

* cmd ref: term: prop->property (plots)
per #1382 (comment)

* cmd ref: more details on metrics index
per #1382 (review)
and #1382 (review)

* cmd ref: more details on plots index
per #1382 (review)
and #1382 (review)

* cmd ref: note about disply props in plots modify
per #1382 (review)
and #1382 (review)

Co-authored-by: Dmitry Petrov <dmitry.petrov@nevesomo.com>
shcheklein pushed a commit to treeverse/dvc.org that referenced this pull request Jun 11, 2020
@jorgeorpinel @dmpetrov
cmd ref: dvc add 1.0 update (#1411)
64182e2 
* cmd ref: add note that move creates dirs

* cmd ref: improve structure of add ref desc.

* grammar: add some commas

* term: checksum -> hash value in dvcignore guide

* style: lower case bullet text

* cmd ref: remove some redundancy in metrics index

* cmd ref: update plots refs synopsis and descriptions
per treeverse/dvc/issues/3924 et al.

* Add plots modify cmd

* typo: CSV->csv

* term: working tree -> workspace
per treeverse/dvc/pull/3914

* cmd ref: couple improvements to add ref
per #1382 (review)
and #1382 (review)

* Update config/prismjs/dvc-commands.js

* cmd ref: update plots modify description

* cmd ref: add plots modify to nav, with a few more improvements

* cmd ref: plots --show-json -> --show-vega
per treeverse/dvc#3891 (comment)

* rename x-lab to x-label

* cmd ref: review descriptions of plots index, show, and diff

* cmd ref: review and update old plots cmds options
per treeverse/dvc#3948 et al.

* cmd ref: a couple more option updates
per #1382 (review)

* cmd ref: emphasize add works with any large file/dir
per #1382 (review)

* cmd ref: updae plots modify top half (definition, description)
per #1382 (review) al.

* cmd ref: improve all plot cmd option descriptions

* Update content/docs/command-reference/plots/modify.md

* cmd ref: review examples (mainly images) in plots modify
per #1382 (comment) et al.

* cmd ref: rephrase info about how data arrays are injected to plot templates
per #1382 (review)

* cmd ref: update info on how targets for for plots show/diff
per #1382 (review)

* cmd ref: double check all plots examples
per #1382 (comment)

* cmd ref: remove info about plots show --select

* cmd ref: update add desc
per #1382 (review)

* cmd ref: re-explain dvc add for dirs
per #1382 (review)

* cmd ref: improve description about targets in plots diff
per #1382 (review)

* cmd ref: make emoji note in plots index
per #1382 (review)

* cmd ref: remove ineffective CSV code block highlighting from plots refs
per #1382 (review)

* get started: improve intro in index

* glossary: remove external deps entry (no need)

* cmd ref: update add for 1.0 (1) up to...
before Examples

* cmd ref: 1.0 updates for add (2) - examples

* cmd ref: remove note about comments in add example
per #1411 (review)

Co-authored-by: Dmitry Petrov <dmitry.petrov@nevesomo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@jorgeorpinel jorgeorpinel jorgeorpinel left review comments

@dmpetrov dmpetrov dmpetrov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

plots: replace --show-json with --show-vega

3 participants

@efiop @jorgeorpinel @dmpetrov