params: read from ParamsDependency if possible (#5550)

* params: read from ParamsDependency if possible

* params: introduce include_untracked flag

* params: include untracked upon targets

* experiments: add option to show tracked params

* fixing test

* params: rename tracked to deps

* params: roll back to default params.yaml behavior

* experiments: add option to show only used params on diff

* Update dvc/command/experiments.py

Co-authored-by: Jorge Orpinel <[email protected]>

* Update dvc/command/experiments.py

Co-authored-by: Jorge Orpinel <[email protected]>

* Update dvc/command/params.py

Co-authored-by: Jorge Orpinel <[email protected]>

Co-authored-by: Jorge Orpinel <[email protected]>

dvc/command/experiments.py

@@ -365,6 +365,7 @@ class CmdExperimentsShow(CmdBase):
                 all_commits=self.args.all_commits,
                 sha_only=self.args.sha,
                 num=self.args.num,
+                param_deps=self.args.param_deps,
             )
 
             if self.args.show_json:
@@ -470,6 +471,7 @@ class CmdExperimentsDiff(CmdBase):
                 a_rev=self.args.a_rev,
                 b_rev=self.args.b_rev,
                 all=self.args.all,
+                param_deps=self.args.param_deps,
             )
 
             if self.args.show_json:
@@ -781,6 +783,12 @@ def add_parser(subparsers, parent_parser):
         help="Exclude the specified params from output table.",
         metavar="<params_list>",
     )
+    experiments_show_parser.add_argument(
+        "--param-deps",
+        action="store_true",
+        default=False,
+        help="Show only params that are stage dependencies.",
+    )
     experiments_show_parser.add_argument(
         "--sort-by",
         help="Sort related experiments by the specified metric or param.",
@@ -866,6 +874,12 @@ def add_parser(subparsers, parent_parser):
         default=False,
         help="Show unchanged metrics/params as well.",
     )
+    experiments_diff_parser.add_argument(
+        "--param-deps",
+        action="store_true",
+        default=False,
+        help="Show only params that are stage dependencies.",
+    )
     experiments_diff_parser.add_argument(
         "--show-json",
         action="store_true",

dvc/command/params.py

@@ -40,6 +40,7 @@ class CmdParamsDiff(CmdBase):
                 b_rev=self.args.b_rev,
                 targets=self.args.targets,
                 all=self.args.all,
+                deps=self.args.deps,
             )
 
             if self.args.show_json:
@@ -111,6 +112,12 @@ def add_parser(subparsers, parent_parser):
         default=False,
         help="Show unchanged params as well.",
     )
+    params_diff_parser.add_argument(
+        "--deps",
+        action="store_true",
+        default=False,
+        help="Show only params that are stage dependencies.",
+    )
     params_diff_parser.add_argument(
         "--show-json",
         action="store_true",

dvc/dependency/param.py

@@ -39,6 +39,7 @@ class ParamsDependency(LocalDependency):
             path
             or os.path.join(stage.repo.root_dir, self.DEFAULT_PARAMS_FILE),
             info=info,
+            fs=stage.repo.fs,
         )
 
     def dumpd(self):
@@ -84,19 +85,34 @@ class ParamsDependency(LocalDependency):
     def status(self):
         return self.workspace_status()
 
-    def read_params(self):
+    def _read(self):
         if not self.exists:
             return {}
 
         suffix = self.path_info.suffix.lower()
         loader = LOADERS[suffix]
         try:
-            config = loader(self.path_info, fs=self.repo.fs)
+            return loader(self.path_info, fs=self.repo.fs)
         except ParseError as exc:
             raise BadParamFileError(
                 f"Unable to read parameters from '{self}'"
             ) from exc
 
+    def read_params_d(self):
+        config = self._read()
+
+        ret = {}
+        for param in self.params:
+            dpath.util.merge(
+                ret,
+                dpath.util.search(config, param, separator="."),
+                separator=".",
+            )
+        return ret
+
+    def read_params(self):
+        config = self._read()
+
         ret = {}
         for param in self.params:
             try:

dvc/repo/experiments/diff.py

@@ -6,7 +6,7 @@ from dvc.utils.diff import format_dict
 logger = logging.getLogger(__name__)
 
 
-def diff(repo, *args, a_rev=None, b_rev=None, **kwargs):
+def diff(repo, *args, a_rev=None, b_rev=None, param_deps=False, **kwargs):
     from dvc.repo.experiments.show import _collect_experiment_commit
 
     if repo.scm.no_commits:
@@ -14,15 +14,17 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs):
 
     if a_rev:
         rev = repo.scm.resolve_rev(a_rev)
-        old = _collect_experiment_commit(repo, rev)
+        old = _collect_experiment_commit(repo, rev, param_deps=param_deps)
     else:
-        old = _collect_experiment_commit(repo, "HEAD")
+        old = _collect_experiment_commit(repo, "HEAD", param_deps=param_deps)
 
     if b_rev:
         rev = repo.scm.resolve_rev(b_rev)
-        new = _collect_experiment_commit(repo, rev)
+        new = _collect_experiment_commit(repo, rev, param_deps=param_deps)
     else:
-        new = _collect_experiment_commit(repo, "workspace")
+        new = _collect_experiment_commit(
+            repo, "workspace", param_deps=param_deps
+        )
 
     with_unchanged = kwargs.pop("all", False)
 

dvc/repo/experiments/show.py

@@ -12,7 +12,9 @@ from dvc.scm.base import SCMError
 logger = logging.getLogger(__name__)
 
 
-def _collect_experiment_commit(repo, rev, stash=False, sha_only=True):
+def _collect_experiment_commit(
+    repo, rev, stash=False, sha_only=True, param_deps=False
+):
     res = defaultdict(dict)
     for rev in repo.brancher(revs=[rev]):
         if rev == "workspace":
@@ -21,8 +23,10 @@ def _collect_experiment_commit(repo, rev, stash=False, sha_only=True):
             commit = repo.scm.resolve_commit(rev)
             res["timestamp"] = datetime.fromtimestamp(commit.commit_time)
 
-        configs = _collect_configs(repo, rev=rev)
-        params = _read_params(repo, configs, rev)
+        params, params_path_infos = _collect_configs(repo, rev=rev)
+        params = _read_params(
+            repo, params, params_path_infos, rev, deps=param_deps
+        )
         if params:
             res["params"] = params
 
@@ -83,6 +87,7 @@ def show(
     all_commits=False,
     sha_only=False,
     num=1,
+    param_deps=False,
 ):
     res = defaultdict(OrderedDict)
 
@@ -110,7 +115,7 @@ def show(
 
     for rev in revs:
         res[rev]["baseline"] = _collect_experiment_commit(
-            repo, rev, sha_only=sha_only
+            repo, rev, sha_only=sha_only, param_deps=param_deps
         )
 
         if rev == "workspace":
@@ -127,14 +132,19 @@ def show(
             ref_info = ExpRefInfo.from_ref(exp_ref)
             assert ref_info.baseline_sha == rev
             _collect_experiment_branch(
-                res[rev], repo, exp_ref, rev, sha_only=sha_only
+                res[rev],
+                repo,
+                exp_ref,
+                rev,
+                sha_only=sha_only,
+                param_deps=param_deps,
             )
 
     # collect queued (not yet reproduced) experiments
     for stash_rev, entry in repo.experiments.stash_revs.items():
         if entry.baseline_rev in revs:
             experiment = _collect_experiment_commit(
-                repo, stash_rev, stash=True
+                repo, stash_rev, stash=True, param_deps=param_deps
             )
             res[entry.baseline_rev][stash_rev] = experiment
 

dvc/repo/params/show.py

@@ -1,6 +1,7 @@
 import logging
 from collections import defaultdict
-from typing import TYPE_CHECKING, List
+from copy import copy
+from typing import TYPE_CHECKING, List, Tuple
 
 from dvc.dependency.param import ParamsDependency
 from dvc.exceptions import DvcException
@@ -27,7 +28,9 @@ def _is_params(dep: "BaseOutput"):
     return isinstance(dep, ParamsDependency)
 
 
-def _collect_configs(repo: "Repo", rev, targets=None) -> List["DvcPath"]:
+def _collect_configs(
+    repo: "Repo", rev, targets=None
+) -> Tuple[List["BaseOutput"], List["DvcPath"]]:
     params, path_infos = collect(
         repo,
         targets=targets or [],
@@ -35,31 +38,45 @@ def _collect_configs(repo: "Repo", rev, targets=None) -> List["DvcPath"]:
         output_filter=_is_params,
         rev=rev,
     )
-    path_infos.extend([p.path_info for p in params])
+    all_path_infos = path_infos + [p.path_info for p in params]
     if not targets:
         default_params = (
             PathInfo(repo.root_dir) / ParamsDependency.DEFAULT_PARAMS_FILE
         )
-        if default_params not in path_infos:
+        if default_params not in all_path_infos:
             path_infos.append(default_params)
-    return list(path_infos)
+    return params, path_infos
 
 
-def _read_params(repo, configs, rev):
-    res = {}
-    for config in configs:
-        if not repo.fs.exists(config):
-            continue
+def _read_path_info(fs, path_info, rev):
+    if not fs.exists(path_info):
+        return None
 
-        suffix = config.suffix.lower()
-        loader = LOADERS[suffix]
-        try:
-            res[str(config)] = loader(config, fs=repo.fs)
-        except ParseError:
-            logger.debug(
-                "failed to read '%s' on '%s'", config, rev, exc_info=True
-            )
-            continue
+    suffix = path_info.suffix.lower()
+    loader = LOADERS[suffix]
+    try:
+        return loader(path_info, fs=fs)
+    except ParseError:
+        logger.debug(
+            "failed to read '%s' on '%s'", path_info, rev, exc_info=True
+        )
+        return None
+
+
+def _read_params(repo, params, params_path_infos, rev, deps=False):
+    res = defaultdict(dict)
+    path_infos = copy(params_path_infos)
+
+    if deps:
+        for param in params:
+            res[str(param.path_info)].update(param.read_params_d())
+    else:
+        path_infos += [param.path_info for param in params]
+
+    for path_info in path_infos:
+        from_path = _read_path_info(repo.fs, path_info, rev)
+        if from_path:
+            res[str(path_info)] = from_path
 
     return res
 
@@ -79,12 +96,12 @@ def _collect_vars(repo, params):
 
 
 @locked
-def show(repo, revs=None, targets=None):
+def show(repo, revs=None, targets=None, deps=False):
     res = {}
 
     for branch in repo.brancher(revs=revs):
-        configs = _collect_configs(repo, branch, targets)
-        params = _read_params(repo, configs, branch)
+        params, params_path_infos = _collect_configs(repo, branch, targets)
+        params = _read_params(repo, params, params_path_infos, branch, deps)
         vars_params = _collect_vars(repo, params)
 
         # NOTE: only those that are not added as a ParamDependency are included

tests/func/params/test_diff.py

@@ -64,19 +64,6 @@ def test_diff_deleted(tmp_dir, scm, dvc):
     }
 
 
-def test_diff_deleted_config(tmp_dir, scm, dvc):
-    tmp_dir.gen("params.yaml", "foo: bar")
-    dvc.run(cmd="echo params.yaml", params=["foo"], single_stage=True)
-    scm.add(["params.yaml", "Dvcfile"])
-    scm.commit("bar")
-
-    (tmp_dir / "params.yaml").unlink()
-
-    assert dvc.params.diff() == {
-        "params.yaml": {"foo": {"old": "bar", "new": None}}
-    }
-
-
 def test_diff_list(tmp_dir, scm, dvc):
     tmp_dir.gen("params.yaml", "foo:\n- bar\n- baz")
     dvc.run(cmd="echo params.yaml", params=["foo"], single_stage=True)

tests/func/params/test_show.py

@@ -80,20 +80,31 @@ def test_show_branch(tmp_dir, scm, dvc):
     }
 
 
-def test_pipeline_tracked_params(tmp_dir, scm, dvc, run_copy):
+def test_pipeline_params(tmp_dir, scm, dvc, run_copy):
     from dvc.dvcfile import PIPELINE_FILE
 
-    tmp_dir.gen({"foo": "foo", "params.yaml": "foo: bar\nxyz: val"})
+    tmp_dir.gen(
+        {"foo": "foo", "params.yaml": "foo: bar\nxyz: val\nabc: ignore"}
+    )
     run_copy("foo", "bar", name="copy-foo-bar", params=["foo,xyz"])
     scm.add(["params.yaml", PIPELINE_FILE])
     scm.commit("add stage")
 
-    tmp_dir.scm_gen("params.yaml", "foo: baz\nxyz: val", commit="baz")
-    tmp_dir.scm_gen("params.yaml", "foo: qux\nxyz: val", commit="qux")
+    tmp_dir.scm_gen(
+        "params.yaml", "foo: baz\nxyz: val\nabc: ignore", commit="baz"
+    )
+    tmp_dir.scm_gen(
+        "params.yaml", "foo: qux\nxyz: val\nabc: ignore", commit="qux"
+    )
 
-    assert dvc.params.show(revs=["master"]) == {
+    assert dvc.params.show(revs=["master"], deps=True) == {
         "master": {"params.yaml": {"foo": "qux", "xyz": "val"}}
     }
+    assert dvc.params.show(revs=["master"]) == {
+        "master": {
+            "params.yaml": {"foo": "qux", "xyz": "val", "abc": "ignore"}
+        }
+    }
 
 
 def test_show_no_repo(tmp_dir):

tests/unit/command/test_experiments.py

@@ -38,6 +38,7 @@ def test_experiments_diff(dvc, scm, mocker):
             "HEAD~10",
             "HEAD~1",
             "--all",
+            "--param-deps",
             "--show-json",
             "--show-md",
             "--old",
@@ -53,7 +54,7 @@ def test_experiments_diff(dvc, scm, 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", all=True, param_deps=True
     )
 
 
@@ -66,6 +67,7 @@ def test_experiments_show(dvc, scm, mocker):
             "--all-branches",
             "--all-commits",
             "--sha",
+            "--param-deps",
             "-n",
             "1",
         ]
@@ -84,6 +86,7 @@ def test_experiments_show(dvc, scm, mocker):
         all_commits=True,
         sha_only=True,
         num=1,
+        param_deps=True,
     )
 
 

tests/unit/command/test_params.py

@@ -18,6 +18,7 @@ def test_params_diff(dvc, mocker):
             "--show-json",
             "--show-md",
             "--no-path",
+            "--deps",
         ]
     )
     assert cli_args.func == CmdParamsDiff
@@ -33,6 +34,21 @@ def test_params_diff(dvc, mocker):
         b_rev="HEAD~1",
         targets=["target"],
         all=True,
+        deps=True,
+    )
+
+
+def test_params_diff_from_cli(dvc, mocker):
+    cli_args = parse_args(["params", "diff"])
+    assert cli_args.func == CmdParamsDiff
+
+    cmd = cli_args.func(cli_args)
+    m = mocker.patch("dvc.repo.params.diff.diff", return_value={})
+
+    assert cmd.run() == 0
+
+    m.assert_called_once_with(
+        cmd.repo, a_rev=None, b_rev=None, all=False, targets=None, deps=False,
     )
 
 

tests/unit/test_params.py

@@ -6,23 +6,28 @@ from dvc.utils.serialize import dumps_yaml
 def test_params_order(tmp_dir, dvc):
     tmp_dir.gen(
         {
-            "params.yaml": dumps_yaml({"p1": 1}),
-            "p1.yaml": dumps_yaml({"p2": 1}),
-            "sub": {"p2.yaml": dumps_yaml({"p3": 1})},
+            "params.yaml": dumps_yaml({"p": 1}),
+            "params1.yaml": dumps_yaml({"p1": 1}),
+            "sub": {"params2.yaml": dumps_yaml({"p2": 1})},
         }
     )
 
-    p2_path = os.path.join("sub", "p2.yaml")
+    params_path = os.path.join("..", "params.yaml")
+    p2_path = os.path.join("sub", "params2.yaml")
     dvc.stage.add(
-        params=[{p2_path: ["p3"]}, {"p1.yaml": ["p2"]}],
+        params=[{p2_path: ["p2"]}, {"params1.yaml": ["p1"]}],
         cmd="cmd1",
         name="stage1",
     )
     with (tmp_dir / "sub").chdir():
-        dvc.stage.add(params=["p1"], cmd="cmd2", name="stage2")
+        dvc.stage.add(params=[{params_path: ["p"]}], cmd="cmd2", name="stage2")
 
     # params are sorted during dumping, therefore p1 is first
-    assert list(dvc.params.show()[""]) == ["p1.yaml", p2_path, "params.yaml"]
+    assert list(dvc.params.show()[""]) == [
+        "params1.yaml",
+        p2_path,
+        "params.yaml",
+    ]
 
 
 def test_repro_unicode(tmp_dir, dvc):