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

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

Merged
merged 1 commit into from
May 28, 2020
Merged

Conversation

efiop
Copy link
Contributor

@efiop efiop commented May 28, 2020

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. πŸ™

Copy link
Member

@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 iterative:master May 28, 2020
@efiop efiop deleted the fix-3890 branch May 28, 2020 11:42
@jorgeorpinel
Copy link
Contributor

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

action="store_true",
default=False,
help="Show output in JSON format.",
help="Show output in VEGA format.",
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
help="Show output in VEGA format.",
help="Show output in Vega format.",

Copy link
Contributor

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
Contributor Author

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
Contributor

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
Contributor Author

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 iterative/dvc.org that referenced this pull request Jun 3, 2020
@jorgeorpinel
Copy link
Contributor

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

I did this in iterative/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
Contributor Author

efiop commented Jun 3, 2020

Thank you @jorgeorpinel !

And I also forgot to update completion scripts πŸ€¦β€β™‚οΈ Will do.

shcheklein pushed a commit to iterative/dvc.org that referenced this pull request Jun 11, 2020
* 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 iterative/dvc/issues/3924 et al.

* Add plots modify cmd

* typo: CSV->csv

* term: working tree -> workspace
per iterative/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 iterative/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 iterative/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 iterative/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 <[email protected]>
shcheklein pushed a commit to iterative/dvc.org that referenced this pull request Jun 11, 2020
* 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 iterative/dvc/issues/3924 et al.

* Add plots modify cmd

* typo: CSV->csv

* term: working tree -> workspace
per iterative/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 iterative/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 iterative/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 <[email protected]>
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: replace --show-json with --show-vega
3 participants