From cb9d3529afb67ff84ae2277fb57aee8e321d87d2 Mon Sep 17 00:00:00 2001 From: David de la Iglesia Castro Date: Wed, 1 Mar 2023 11:25:35 +0100 Subject: [PATCH] plots: Remove `--from-config` --- dvc/commands/plots.py | 11 --------- dvc/repo/plots/__init__.py | 40 +++++++------------------------- tests/func/plots/test_show.py | 27 +++++---------------- tests/unit/command/test_plots.py | 5 ---- 4 files changed, 14 insertions(+), 69 deletions(-) diff --git a/dvc/commands/plots.py b/dvc/commands/plots.py index 3d6485733..02adba8ca 100644 --- a/dvc/commands/plots.py +++ b/dvc/commands/plots.py @@ -76,10 +76,6 @@ class CmdPlots(CmdBase): props = {p: getattr(self.args, p) for p in PLOT_PROPS} return {k: v for k, v in props.items() if v is not None} - def _config_files(self): - if self.args.from_config: - return {self.args.from_config} - def _html_template_path(self): html_template_path = self.args.html_template if not html_template_path: @@ -114,7 +110,6 @@ class CmdPlots(CmdBase): plots_data = self._func( targets=self.args.targets, props=self._props(), - config_files=self._config_files(), ) if not plots_data: @@ -412,9 +407,3 @@ def _add_ui_arguments(parser): help="Custom HTML template for VEGA visualization.", metavar="", ) - parser.add_argument( - "--from-config", - default=None, - metavar="", - help=argparse.SUPPRESS, - ) diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index 41cef06d3..cf36d36c3 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -74,7 +74,6 @@ class Plots: recursive: bool = False, onerror: Optional[Callable] = None, props: Optional[Dict] = None, - config_files: Optional[Set[str]] = None, ) -> Iterator[Dict]: """Collects plots definitions and data sources. @@ -131,7 +130,6 @@ class Plots: targets=targets, revision=rev, onerror=onerror, - config_files=config_files, props=props, ) if definitions: @@ -186,7 +184,6 @@ class Plots: props=None, recursive=False, onerror=None, - config_files: Optional[Set[str]] = None, ): if onerror is None: onerror = onerror_collect @@ -198,7 +195,6 @@ class Plots: recursive, onerror=onerror, props=props, - config_files=config_files, ): _resolve_data_sources(data) result.update(data) @@ -482,7 +478,6 @@ def _collect_pipeline_files(repo, targets: List[str], props, onerror=None): def _collect_definitions( repo: "Repo", targets=None, - config_files: Optional[Set[str]] = None, props: Optional[Dict] = None, onerror: Optional[Callable] = None, **kwargs, @@ -494,34 +489,15 @@ def _collect_definitions( fs = DVCFileSystem(repo=repo) - if not config_files: - dpath.merge( - result, - _collect_pipeline_files(repo, targets, props, onerror=onerror), - ) + dpath.merge( + result, + _collect_pipeline_files(repo, targets, props, onerror=onerror), + ) - if targets or (not targets and not config_files): - dpath.merge( - result, - _collect_output_plots(repo, targets, props, onerror=onerror), - ) - - if config_files: - for path in config_files: - definitions = parse(fs, path).get("data", {}) - if definitions: - resolved = _resolve_definitions( - repo.dvcfs, - targets, - props, - path, - definitions, - onerror=onerror, - ) - dpath.merge( - result, - {path: resolved}, - ) + dpath.merge( + result, + _collect_output_plots(repo, targets, props, onerror=onerror), + ) for target in targets: if not result or fs.exists(target): diff --git a/tests/func/plots/test_show.py b/tests/func/plots/test_show.py index 967304a79..4a3d36a1d 100644 --- a/tests/func/plots/test_show.py +++ b/tests/func/plots/test_show.py @@ -9,7 +9,7 @@ from dvc.repo import Repo from dvc.repo.plots import PlotMetricTypeError from dvc.utils import onerror_collect from dvc.utils.fs import remove -from dvc.utils.serialize import EncodingError, YAMLFileCorruptedError +from dvc.utils.serialize import EncodingError, YAMLFileCorruptedError, modify_yaml from tests.utils.plots import get_plot @@ -351,14 +351,11 @@ def test_collect_non_existing_dir(tmp_dir, dvc, run_copy_metrics): ), ], ) -@pytest.mark.parametrize("separate_config", [True, False]) -def test_load_from_config( +def test_top_level_plots( tmp_dir, dvc, plot_config, expected_datafiles, - separate_config, - run_copy_metrics, ): data = { "data1.json": [ @@ -377,23 +374,11 @@ def test_load_from_config( os.makedirs(dirname) (tmp_dir / filename).dump_json(content, sort_keys=True) - config_files = None - if separate_config: - (tmp_dir / "plot_config.json").dump_json(plot_config, sort_keys=True) - config_file = "plot_config.json" - config_files = {config_file} - else: - # TODO we need that to create any stage, as dvc.yaml plots - # collections bases on existing stages - fix collection - run_copy_metrics("data1.json", "copy.json", name="train") + config_file = "dvc.yaml" + with modify_yaml(config_file) as dvcfile_content: + dvcfile_content["plots"] = plot_config - from dvc.utils.serialize import modify_yaml - - config_file = "dvc.yaml" - with modify_yaml(config_file) as dvcfile_content: - dvcfile_content["plots"] = plot_config - - result = dvc.plots.show(config_files=config_files) + result = dvc.plots.show() assert plot_config == get_plot( result, "workspace", typ="definitions", file=config_file diff --git a/tests/unit/command/test_plots.py b/tests/unit/command/test_plots.py index d020bf78a..35e3c21e6 100644 --- a/tests/unit/command/test_plots.py +++ b/tests/unit/command/test_plots.py @@ -53,8 +53,6 @@ def test_plots_diff(dvc, mocker, plots_data): "HEAD", "tag1", "tag2", - "--from-config", - "path_to_config", ] ) assert cli_args.func == CmdPlotsDiff @@ -78,7 +76,6 @@ def test_plots_diff(dvc, mocker, plots_data): "y_label": "y_title", }, experiment=True, - config_files={"path_to_config"}, ) render_mock.assert_not_called() @@ -104,7 +101,6 @@ def test_plots_show_vega(dvc, mocker, plots_data): m = mocker.patch( "dvc.repo.plots.Plots.show", return_value=plots_data, - config_files=None, ) render_mock = mocker.patch("dvc_render.render_html", return_value="html_path") @@ -113,7 +109,6 @@ def test_plots_show_vega(dvc, mocker, plots_data): m.assert_called_once_with( targets=["datafile"], props={"template": "template", "header": False}, - config_files=None, ) render_mock.assert_not_called()