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

dvc plots: allow for setting output directory via config #7911

Merged

Conversation

ykasimov
Copy link

@ykasimov ykasimov commented Jun 17, 2022

  • [x ] ❗ 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.

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

This PR allows users to set output directory for dvc plots show via dvc config. I added a test covering this scenario.
To test run the following script:

#!/bin/bash

set -ex
rm -rf repo
mkdir repo
pushd repo

echo '[{"first_val": 100, "val": 2}, {"first_val": 200, "val": 3}]' >> metric.json

dvc config plots.out_dir config_dir

dvc plots show metric.json

dvc plots show -o arg_dir metric.json

@ykasimov ykasimov requested a review from a team as a code owner June 17, 2022 11:55
@ykasimov ykasimov requested a review from pmrowla June 17, 2022 11:55
Copy link
Contributor

@pared pared left a comment

Choose a reason for hiding this comment

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

Looks good, some minor things related to tests.

tests/func/plots/test_show.py Outdated Show resolved Hide resolved
tests/func/plots/test_show.py Outdated Show resolved Hide resolved
@ykasimov ykasimov requested a review from pared June 17, 2022 14:01
Copy link
Contributor

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

PΜΆeΜΆrΜΆhΜΆaΜΆpΜΆsΜΆ ΜΆIΜΆ ΜΆaΜΆmΜΆ ΜΆoΜΆvΜΆeΜΆrΜΆtΜΆhΜΆiΜΆnΜΆkΜΆiΜΆnΜΆgΜΆ ΜΆaΜΆnΜΆdΜΆ ΜΆiΜΆtΜΆ'ΜΆsΜΆ ΜΆjΜΆuΜΆsΜΆtΜΆ ΜΆaΜΆ ΜΆmΜΆaΜΆtΜΆtΜΆeΜΆrΜΆ ΜΆoΜΆfΜΆ ΜΆaΜΆdΜΆdΜΆiΜΆnΜΆgΜΆ ΜΆeΜΆxΜΆaΜΆmΜΆpΜΆlΜΆeΜΆsΜΆ ΜΆiΜΆnΜΆ ΜΆdΜΆoΜΆcΜΆsΜΆ.ΜΆ ΜΆSΜΆtΜΆiΜΆlΜΆlΜΆ,ΜΆ ΜΆwΜΆeΜΆ ΜΆaΜΆlΜΆlΜΆoΜΆwΜΆ ΜΆsΜΆtΜΆoΜΆrΜΆiΜΆnΜΆgΜΆ ΜΆaΜΆnΜΆ ΜΆuΜΆnΜΆrΜΆeΜΆsΜΆoΜΆlΜΆvΜΆeΜΆdΜΆ ΜΆpΜΆaΜΆtΜΆhΜΆ ΜΆiΜΆnΜΆ ΜΆΜΆ.ΜΆdΜΆvΜΆcΜΆ/ΜΆcΜΆoΜΆnΜΆfΜΆiΜΆgΜΆΜΆ ΜΆsΜΆoΜΆ ΜΆuΜΆsΜΆeΜΆrΜΆsΜΆ ΜΆcΜΆaΜΆlΜΆlΜΆiΜΆnΜΆgΜΆ ΜΆfΜΆrΜΆoΜΆmΜΆ ΜΆdΜΆiΜΆfΜΆfΜΆeΜΆrΜΆeΜΆnΜΆtΜΆ ΜΆCΜΆWΜΆDΜΆ ΜΆwΜΆiΜΆlΜΆlΜΆ ΜΆsΜΆaΜΆvΜΆeΜΆ ΜΆpΜΆlΜΆoΜΆtΜΆsΜΆ ΜΆtΜΆoΜΆ ΜΆdΜΆiΜΆfΜΆfΜΆeΜΆrΜΆeΜΆnΜΆtΜΆ ΜΆfΜΆoΜΆlΜΆdΜΆeΜΆrΜΆsΜΆ.ΜΆ

Looks like we already do this for other path config options (i.e. html template path)

@daavoo daavoo added A: plots Related to the plots feature is a feature labels Jun 20, 2022
@daavoo daavoo force-pushed the 7816_set_output_directory_via_config branch from 5495e3b to a38d9e1 Compare June 20, 2022 09:30
@pared
Copy link
Contributor

pared commented Jun 20, 2022

@ykasimov @daavoo has rebased the PR, could you squash the commits and include which issue it fixes in the commit message?

@ykasimov ykasimov force-pushed the 7816_set_output_directory_via_config branch from 0b80e8f to 1d98b9b Compare June 20, 2022 15:44
@ykasimov
Copy link
Author

All done. πŸ‘

@daavoo daavoo merged commit 735b563 into iterative:main Jun 20, 2022
daavoo added a commit to iterative/dvc.org that referenced this pull request Jul 5, 2022
daavoo added a commit to iterative/dvc.org that referenced this pull request Jul 22, 2022
* config: Add `plots.out_dir`.

Per iterative/dvc#7911

* Update content/docs/command-reference/config.md

* Restyled by prettier (#3760)

Co-authored-by: Restyled.io <commits@restyled.io>

* Updates

* Format

* Apply suggestions from code review

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <commits@restyled.io>
@ykasimov ykasimov deleted the 7816_set_output_directory_via_config branch October 6, 2022 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: plots Related to the plots feature is a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dvc plots show: Allow for setting output directory via config or dvc.yaml
3 participants