params: introduce --targets (#5122)

* params: add targets option

* fixup

* remove unnecessary changes

* fix isort

* params: diff: add test for --targets

* tests: roll back metric/plots unification, introduce collect test

* metrics/plots: refactoring

* missing plots test added

dvc/command/params.py

@@ -2,6 +2,7 @@ import argparse
 import logging
 from collections import OrderedDict
 
+from dvc.command import completion
 from dvc.command.base import CmdBase, append_doc_link, fix_subparsers
 from dvc.exceptions import DvcException
 
@@ -30,11 +31,14 @@ def _show_diff(diff, markdown=False, no_path=False):
 
 
 class CmdParamsDiff(CmdBase):
+    UNINITIALIZED = True
+
     def run(self):
         try:
             diff = self.repo.params.diff(
                 a_rev=self.args.a_rev,
                 b_rev=self.args.b_rev,
+                targets=self.args.targets,
                 all=self.args.all,
             )
 
@@ -91,6 +95,12 @@ def add_parser(subparsers, parent_parser):
         nargs="?",
         help=("New Git commit to compare (defaults to the current workspace)"),
     )
+    params_diff_parser.add_argument(
+        "--targets",
+        nargs="*",
+        help="Limit command scope to these params files.",
+        metavar="<path>",
+    ).complete = completion.FILE
     params_diff_parser.add_argument(
         "--all",
         action="store_true",

dvc/exceptions.py

@@ -1,4 +1,6 @@
 """Exceptions raised by the dvc."""
+from typing import List
+
 from funcy import first
 
 from dvc.utils import error_link, format_link, relpath
@@ -170,11 +172,11 @@ class BadMetricError(DvcException):
         )
 
 
-class NoMetricsError(DvcException):
+class MetricsError(DvcException):
     pass
 
 
-class NoMetricsParsedError(NoMetricsError):
+class NoMetricsParsedError(MetricsError):
     def __init__(self, command):
         super().__init__(
             f"Could not parse {command} files. Use `-v` option to see more "
@@ -182,7 +184,7 @@ class NoMetricsParsedError(NoMetricsError):
         )
 
 
-class NoMetricsFoundError(NoMetricsError):
+class NoMetricsFoundError(MetricsError):
     def __init__(self, command, run_options):
         super().__init__(
             f"No {command} files in this repository. "
@@ -191,6 +193,15 @@ class NoMetricsFoundError(NoMetricsError):
         )
 
 
+class MetricDoesNotExistError(MetricsError):
+    def __init__(self, targets: List[str]):
+        if len(targets) == 1:
+            msg = "File: '{}' does not exist."
+        else:
+            msg = "Files: '{}' do not exist."
+        super().__init__(msg.format(", ".join(targets)))
+
+
 class RecursiveAddingWhileUsingFilename(DvcException):
     def __init__(self):
         super().__init__(

dvc/repo/metrics/diff.py

@@ -1,4 +1,4 @@
-from dvc.exceptions import NoMetricsError
+from dvc.exceptions import MetricsError
 from dvc.utils.diff import diff as _diff
 from dvc.utils.diff import format_dict
 
@@ -7,7 +7,7 @@ def _get_metrics(repo, *args, revs=None, **kwargs):
     try:
         metrics = repo.metrics.show(*args, **kwargs, revs=revs)
         return metrics
-    except NoMetricsError:
+    except MetricsError:
         return {}
 
 

dvc/repo/metrics/show.py

@@ -1,6 +1,10 @@
 import logging
 
-from dvc.exceptions import NoMetricsFoundError, NoMetricsParsedError
+from dvc.exceptions import (
+    MetricDoesNotExistError,
+    NoMetricsFoundError,
+    NoMetricsParsedError,
+)
 from dvc.repo import locked
 from dvc.repo.collect import collect
 from dvc.scm.base import SCMError
@@ -105,6 +109,8 @@ def show(
     if not res:
         if metrics_found:
             raise NoMetricsParsedError("metrics")
+        elif targets:
+            raise MetricDoesNotExistError(targets)
         else:
             raise NoMetricsFoundError("metrics", "-m/-M")
 

dvc/repo/params/show.py

@@ -7,6 +7,7 @@ from dvc.exceptions import DvcException
 from dvc.path_info import PathInfo
 from dvc.repo import locked
 from dvc.repo.collect import collect
+from dvc.scm.base import SCMError
 from dvc.stage import PipelineStage
 from dvc.utils.serialize import LOADERS, ParseError
 
@@ -25,11 +26,20 @@ def _is_params(dep: "BaseOutput"):
     return isinstance(dep, ParamsDependency)
 
 
-def _collect_configs(repo: "Repo", rev) -> List[PathInfo]:
-    params, _ = collect(repo, deps=True, output_filter=_is_params, rev=rev)
-    configs = {p.path_info for p in params}
-    configs.add(PathInfo(repo.root_dir) / ParamsDependency.DEFAULT_PARAMS_FILE)
-    return list(configs)
+def _collect_configs(repo: "Repo", rev, targets=None) -> List[PathInfo]:
+    params, path_infos = collect(
+        repo,
+        targets=targets or [],
+        deps=True,
+        output_filter=_is_params,
+        rev=rev,
+    )
+    path_infos.update({p.path_info for p in params})
+    if not targets:
+        path_infos.add(
+            PathInfo(repo.root_dir) / ParamsDependency.DEFAULT_PARAMS_FILE
+        )
+    return list(path_infos)
 
 
 def _read_params(repo, configs, rev):
@@ -66,11 +76,11 @@ def _collect_vars(repo, params):
 
 
 @locked
-def show(repo, revs=None):
+def show(repo, revs=None, targets=None):
     res = {}
 
     for branch in repo.brancher(revs=revs):
-        configs = _collect_configs(repo, branch)
+        configs = _collect_configs(repo, branch, targets)
         params = _read_params(repo, configs, branch)
         vars_params = _collect_vars(repo, params)
 
@@ -87,8 +97,10 @@ def show(repo, revs=None):
     # Hide workspace params if they are the same as in the active branch
     try:
         active_branch = repo.scm.active_branch()
-    except TypeError:
-        pass  # Detached head
+    except (TypeError, SCMError):
+        # TypeError - detached head
+        # SCMError - no repo case
+        pass
     else:
         if res.get("workspace") == res.get(active_branch):
             res.pop("workspace", None)

dvc/repo/plots/__init__.py

@@ -4,6 +4,7 @@ from funcy import cached_property, first, project
 
 from dvc.exceptions import (
     DvcException,
+    MetricDoesNotExistError,
     NoMetricsFoundError,
     NoMetricsParsedError,
 )
@@ -96,7 +97,6 @@ class Plots:
         return result
 
     def show(self, targets=None, revs=None, props=None, templates=None):
-        from .data import NoMetricInHistoryError
 
         data = self.collect(targets, revs)
 
@@ -105,7 +105,7 @@ class Plots:
         for target in targets:
             rpath = relpath(target, self.repo.root_dir)
             if not any("data" in d[rpath] for d in data.values()):
-                raise NoMetricInHistoryError(target)
+                raise MetricDoesNotExistError([target])
 
         # No data at all is a special error with a special message
         if not data:

dvc/repo/plots/data.py

@@ -26,11 +26,6 @@ class PlotDataStructureError(DvcException):
         )
 
 
-class NoMetricInHistoryError(DvcException):
-    def __init__(self, path):
-        super().__init__(f"Could not find '{path}'.")
-
-
 class PlotParsingError(ParseError):
     def __init__(self, path, revision):
         self.path = path

tests/func/metrics/test_common.py

@@ -1,108 +0,0 @@
-import logging
-
-import pytest
-
-from dvc.exceptions import (
-    NoMetricsFoundError,
-    NoMetricsParsedError,
-    OverlappingOutputPathsError,
-)
-from dvc.path_info import PathInfo
-from dvc.utils.fs import remove
-from dvc.utils.serialize import dump_yaml, modify_yaml
-from tests.func.metrics.utils import _write_json
-
-
-@pytest.mark.parametrize(
-    "diff, metric_value",
-    (
-        (
-            lambda repo, target, rev: repo.metrics.diff(
-                targets=[target], a_rev=rev
-            ),
-            {"m": 1},
-        ),
-        (
-            lambda repo, target, rev: repo.plots.diff(
-                targets=[target], revs=[rev]
-            ),
-            [{"m": 1}, {"m": 2}],
-        ),
-    ),
-)
-def test_diff_no_file_on_target_rev(
-    tmp_dir, scm, dvc, caplog, diff, metric_value
-):
-    with tmp_dir.branch("new_branch", new=True):
-        _write_json(tmp_dir, metric_value, "metric.json")
-
-        with caplog.at_level(logging.WARNING, "dvc"):
-            diff(dvc, "metric.json", "master")
-
-    assert "'metric.json' was not found at: 'master'." in caplog.text
-
-
-@pytest.mark.parametrize(
-    "show, malformed_metric",
-    (
-        (lambda repo, target: repo.metrics.show(targets=[target]), '{"m": 1'),
-        (
-            lambda repo, target: repo.plots.show(targets=[target]),
-            '[{"m": 1}, {"m": 2}',
-        ),
-    ),
-)
-def test_show_malformed_metric(
-    tmp_dir, scm, dvc, caplog, show, malformed_metric
-):
-    tmp_dir.gen("metric.json", malformed_metric)
-
-    with pytest.raises(NoMetricsParsedError):
-        show(dvc, "metric.json")
-
-
-@pytest.mark.parametrize(
-    "show",
-    (lambda repo: repo.metrics.show(), lambda repo: repo.plots.show(),),
-)
-def test_show_no_metrics_files(tmp_dir, dvc, caplog, show):
-    with pytest.raises(NoMetricsFoundError):
-        show(dvc)
-
-
-@pytest.mark.parametrize("clear_before_run", [True, False])
-@pytest.mark.parametrize("typ", ["metrics", "plots"])
-def test_metrics_show_overlap(
-    tmp_dir, dvc, run_copy_metrics, clear_before_run, typ
-):
-    data_dir = PathInfo("data")
-    (tmp_dir / data_dir).mkdir()
-
-    outs = {typ: [str(data_dir / "m1.yaml")]}
-    dump_yaml(data_dir / "m1_temp.yaml", {"a": {"b": {"c": 2, "d": 1}}})
-    run_copy_metrics(
-        str(data_dir / "m1_temp.yaml"),
-        str(data_dir / "m1.yaml"),
-        single_stage=False,
-        commit=f"add m1 {typ}",
-        name="cp-m1",
-        **outs,
-    )
-    with modify_yaml("dvc.yaml") as d:
-        # trying to make an output overlaps error
-        d["stages"]["corrupted-stage"] = {
-            "cmd": "mkdir data",
-            "outs": ["data"],
-        }
-
-    # running by clearing and not clearing stuffs
-    # so as it works even for optimized cases
-    if clear_before_run:
-        remove(data_dir)
-        remove(dvc.cache.local.cache_dir)
-
-    dvc._reset()
-
-    show = dvc.metrics.show if typ == "metrics" else dvc.plots.show
-    with pytest.raises(OverlappingOutputPathsError):
-        show()

tests/func/metrics/test_show.py

@@ -2,8 +2,16 @@ import os
 
 import pytest
 
+from dvc.exceptions import (
+    MetricDoesNotExistError,
+    NoMetricsFoundError,
+    NoMetricsParsedError,
+    OverlappingOutputPathsError,
+)
+from dvc.path_info import PathInfo
 from dvc.repo import Repo
 from dvc.utils.fs import remove
+from dvc.utils.serialize import dump_yaml, modify_yaml
 
 
 def test_show_simple(tmp_dir, dvc, run_copy_metrics):
@@ -166,3 +174,55 @@ def test_show_no_repo(tmp_dir):
     dvc = Repo(uninitialized=True)
 
     dvc.metrics.show(targets=["metrics.json"])
+
+
+def test_show_malformed_metric(tmp_dir, scm, dvc, caplog):
+    tmp_dir.gen("metric.json", '{"m":1')
+
+    with pytest.raises(NoMetricsParsedError):
+        dvc.metrics.show(targets=["metric.json"])
+
+
+def test_metrics_show_no_target(tmp_dir, dvc):
+    with pytest.raises(MetricDoesNotExistError):
+        dvc.metrics.show(targets=["metrics.json"])
+
+
+def test_show_no_metrics_files(tmp_dir, dvc, caplog):
+    with pytest.raises(NoMetricsFoundError):
+        dvc.metrics.show()
+
+
+@pytest.mark.parametrize("clear_before_run", [True, False])
+def test_metrics_show_overlap(
+    tmp_dir, dvc, run_copy_metrics, clear_before_run
+):
+    data_dir = PathInfo("data")
+    (tmp_dir / data_dir).mkdir()
+
+    dump_yaml(data_dir / "m1_temp.yaml", {"a": {"b": {"c": 2, "d": 1}}})
+    run_copy_metrics(
+        str(data_dir / "m1_temp.yaml"),
+        str(data_dir / "m1.yaml"),
+        single_stage=False,
+        commit="add m1",
+        name="cp-m1",
+        metrics=[str(data_dir / "m1.yaml")],
+    )
+    with modify_yaml("dvc.yaml") as d:
+        # trying to make an output overlaps error
+        d["stages"]["corrupted-stage"] = {
+            "cmd": "mkdir data",
+            "outs": ["data"],
+        }
+
+    # running by clearing and not clearing stuffs
+    # so as it works even for optimized cases
+    if clear_before_run:
+        remove(data_dir)
+        remove(dvc.cache.local.cache_dir)
+
+    dvc._reset()
+
+    with pytest.raises(OverlappingOutputPathsError):
+        dvc.metrics.show()

tests/func/params/test_diff.py

@@ -189,3 +189,46 @@ def test_vars_shows_on_params_diff(tmp_dir, scm, dvc):
                 "vars.model1.epoch": {"new": 20, "old": 15, "diff": 5},
             }
         }
+
+
+def test_diff_targeted(tmp_dir, scm, dvc, run_copy):
+    from dvc.dvcfile import PIPELINE_FILE
+
+    tmp_dir.gen(
+        {
+            "foo": "foo",
+            "params.yaml": "foo: bar",
+            "other_params.yaml": "xyz: val",
+        }
+    )
+    run_copy(
+        "foo",
+        "bar",
+        name="copy-foo-bar",
+        params=["foo", "other_params.yaml:xyz"],
+    )
+
+    scm.add(["params.yaml", "other_params.yaml", PIPELINE_FILE])
+    scm.commit("add stage")
+
+    tmp_dir.scm_gen(
+        {"params.yaml": "foo: baz", "other_params.yaml": "xyz: val2"},
+        commit="baz",
+    )
+    tmp_dir.scm_gen(
+        {"params.yaml": "foo: qux", "other_params.yaml": "xyz: val3"},
+        commit="qux",
+    )
+
+    assert dvc.params.diff(a_rev="HEAD~2") == {
+        "params.yaml": {"foo": {"old": "bar", "new": "qux"}},
+        "other_params.yaml": {"xyz": {"old": "val", "new": "val3"}},
+    }
+
+    assert dvc.params.diff(a_rev="HEAD~2", targets=["params.yaml"]) == {
+        "params.yaml": {"foo": {"old": "bar", "new": "qux"}}
+    }
+
+    assert dvc.params.diff(a_rev="HEAD~2", targets=["other_params.yaml"]) == {
+        "other_params.yaml": {"xyz": {"old": "val", "new": "val3"}},
+    }

tests/func/params/test_show.py

@@ -1,5 +1,6 @@
 import pytest
 
+from dvc.repo import Repo
 from dvc.repo.params.show import NoParamsError
 
 
@@ -93,3 +94,13 @@ def test_pipeline_tracked_params(tmp_dir, scm, dvc, run_copy):
     assert dvc.params.show(revs=["master"]) == {
         "master": {"params.yaml": {"foo": "qux", "xyz": "val"}}
     }
+
+
+def test_show_no_repo(tmp_dir):
+    tmp_dir.gen({"foo": "foo", "params_file.yaml": "foo: bar\nxyz: val"})
+
+    dvc = Repo(uninitialized=True)
+
+    assert dvc.params.show(targets=["params_file.yaml"]) == {
+        "": {"params_file.yaml": {"foo": "bar", "xyz": "val"}}
+    }

tests/func/plots/test_diff.py

@@ -1,7 +1,7 @@
 import json
 
 from dvc.repo.plots.data import PlotData
-from tests.func.metrics.utils import _write_json
+from tests.func.plots.utils import _write_json
 
 
 def test_diff_dirty(tmp_dir, scm, dvc, run_copy_metrics):

tests/func/plots/test_modify.py

@@ -4,7 +4,7 @@ from dvc.dvcfile import PIPELINE_LOCK
 from dvc.repo.plots import PropsNotFoundError
 from dvc.repo.plots.template import TemplateNotFoundError
 from dvc.utils import relpath
-from tests.func.metrics.utils import _write_json
+from tests.func.plots.utils import _write_json
 
 
 def test_plots_modify_existing_template(

tests/func/plots/test_show.py

@@ -6,11 +6,17 @@ from collections import OrderedDict
 
 import pytest
 
+from dvc.exceptions import (
+    MetricDoesNotExistError,
+    NoMetricsFoundError,
+    NoMetricsParsedError,
+    OverlappingOutputPathsError,
+)
 from dvc.main import main
+from dvc.path_info import PathInfo
 from dvc.repo import Repo
 from dvc.repo.plots.data import (
     JSONPlotData,
-    NoMetricInHistoryError,
     PlotData,
     PlotMetricTypeError,
     YAMLPlotData,
@@ -21,8 +27,8 @@ from dvc.repo.plots.template import (
     TemplateNotFoundError,
 )
 from dvc.utils.fs import remove
-from dvc.utils.serialize import dump_yaml, dumps_yaml
-from tests.func.metrics.utils import _write_csv, _write_json
+from dvc.utils.serialize import dump_yaml, dumps_yaml, modify_yaml
+from tests.func.plots.utils import _write_csv, _write_json
 
 
 def test_plot_csv_one_column(tmp_dir, scm, dvc, run_copy_metrics):
@@ -399,15 +405,15 @@ def test_throw_on_no_metric_at_all(tmp_dir, scm, dvc, caplog):
     tmp_dir.gen("some_file", "make repo dirty")
 
     caplog.clear()
-    with pytest.raises(NoMetricInHistoryError) as error, caplog.at_level(
+    with pytest.raises(MetricDoesNotExistError) as error, caplog.at_level(
         logging.WARNING, "dvc"
     ):
-        dvc.plots.show(targets="metric.json", revs=["v1"])
+        dvc.plots.show(targets="plot.json", revs=["v1"])
 
         # do not warn if none found
         assert len(caplog.messages) == 0
 
-    assert str(error.value) == "Could not find 'metric.json'."
+    assert str(error.value) == "File: 'plot.json' does not exist."
 
 
 def test_custom_template(tmp_dir, scm, dvc, custom_template, run_copy_metrics):
@@ -704,3 +710,53 @@ def test_show_from_subdir(tmp_dir, dvc, caplog):
 
     assert f"file://{str(subdir)}" in caplog.text
     assert (subdir / "plots.html").exists()
+
+
+def test_show_malformed_plots(tmp_dir, scm, dvc, caplog):
+    tmp_dir.gen("plot.json", '[{"m":1]')
+
+    with pytest.raises(NoMetricsParsedError):
+        dvc.metrics.show(targets=["plot.json"])
+
+
+def test_plots_show_no_target(tmp_dir, dvc):
+    with pytest.raises(MetricDoesNotExistError):
+        dvc.plots.show(targets=["plot.json"])
+
+
+def test_show_no_plots_files(tmp_dir, dvc, caplog):
+    with pytest.raises(NoMetricsFoundError):
+        dvc.metrics.show()
+
+
+@pytest.mark.parametrize("clear_before_run", [True, False])
+def test_plots_show_overlap(tmp_dir, dvc, run_copy_metrics, clear_before_run):
+    data_dir = PathInfo("data")
+    (tmp_dir / data_dir).mkdir()
+
+    dump_yaml(data_dir / "m1_temp.yaml", {"a": {"b": {"c": 2, "d": 1}}})
+    run_copy_metrics(
+        str(data_dir / "m1_temp.yaml"),
+        str(data_dir / "m1.yaml"),
+        single_stage=False,
+        commit="add m1",
+        name="cp-m1",
+        plots=[str(data_dir / "m1.yaml")],
+    )
+    with modify_yaml("dvc.yaml") as d:
+        # trying to make an output overlaps error
+        d["stages"]["corrupted-stage"] = {
+            "cmd": "mkdir data",
+            "outs": ["data"],
+        }
+
+    # running by clearing and not clearing stuffs
+    # so as it works even for optimized cases
+    if clear_before_run:
+        remove(data_dir)
+        remove(dvc.cache.local.cache_dir)
+
+    dvc._reset()
+
+    with pytest.raises(OverlappingOutputPathsError):
+        dvc.plots.show()

tests/unit/command/test_params.py

@@ -12,6 +12,8 @@ def test_params_diff(dvc, mocker):
             "diff",
             "HEAD~10",
             "HEAD~1",
+            "--targets",
+            "target",
             "--all",
             "--show-json",
             "--show-md",
@@ -26,7 +28,11 @@ def test_params_diff(dvc, mocker):
     assert cmd.run() == 0
 
     m.assert_called_once_with(
-        cmd.repo, a_rev="HEAD~10", b_rev="HEAD~1", all=True,
+        cmd.repo,
+        a_rev="HEAD~10",
+        b_rev="HEAD~1",
+        targets=["target"],
+        all=True,
     )
 
 

tests/unit/test_collect.py

@@ -0,0 +1,10 @@
+import logging
+
+from dvc.repo.collect import collect
+
+
+def test_no_file_on_target_rev(tmp_dir, scm, dvc, caplog):
+    with caplog.at_level(logging.WARNING, "dvc"):
+        collect(dvc, targets=["file.yaml"], rev="current_branch")
+
+    assert "'file.yaml' was not found at: 'current_branch'." in caplog.text