params: diff: introduce --all (#3631)

* params: diff: introduce --all

Makes `dvc params diff` also show unchanged params.

* params: diff: show all present parameters

dvc/command/params.py

@@ -28,7 +28,9 @@ class CmdParamsDiff(CmdBase):
     def run(self):
         try:
             diff = self.repo.params.diff(
-                a_rev=self.args.a_rev, b_rev=self.args.b_rev,
+                a_rev=self.args.a_rev,
+                b_rev=self.args.b_rev,
+                all=self.args.all,
             )
 
             if self.args.show_json:
@@ -84,6 +86,12 @@ def add_parser(subparsers, parent_parser):
         nargs="?",
         help=("New Git commit to compare (defaults to the current workspace)"),
     )
+    params_diff_parser.add_argument(
+        "--all",
+        action="store_true",
+        default=False,
+        help="Show unchanged params as well.",
+    )
     params_diff_parser.add_argument(
         "--show-json",
         action="store_true",

dvc/repo/params/diff.py

@@ -24,7 +24,11 @@ def _format(params):
 
 
 def diff(repo, *args, a_rev=None, b_rev=None, **kwargs):
+    with_unchanged = kwargs.pop("all", False)
+
     old = _get_params(repo, *args, **kwargs, rev=(a_rev or "HEAD"))
     new = _get_params(repo, *args, **kwargs, rev=b_rev)
 
-    return dvc.utils.diff.diff(_format(old), _format(new))
+    return dvc.utils.diff.diff(
+        _format(old), _format(new), with_unchanged=with_unchanged
+    )

dvc/repo/params/show.py

@@ -1,42 +1,46 @@
-import copy
+import yaml
+import logging
 
 from dvc.repo import locked
+from dvc.path_info import PathInfo
+from dvc.compat import fspath_py35
 from dvc.exceptions import DvcException
 from dvc.dependency.param import DependencyPARAMS
 
 
+logger = logging.getLogger(__name__)
+
+
 class NoParamsError(DvcException):
     pass
 
 
-def _collect_params(repo):
-    configs = {}
+def _collect_configs(repo):
+    configs = set()
+    configs.add(PathInfo(repo.root_dir) / DependencyPARAMS.DEFAULT_PARAMS_FILE)
     for stage in repo.stages:
         for dep in stage.deps:
             if not isinstance(dep, DependencyPARAMS):
                 continue
 
-            if dep.path_info not in configs.keys():
-                configs[dep.path_info] = copy.copy(dep)
-                continue
-
-            params = set(configs[dep.path_info].params)
-            params.update(set(dep.params))
-            configs[dep.path_info].params = list(params)
-
-    return configs.values()
+            configs.add(dep.path_info)
+    return list(configs)
 
 
-def _read_params(deps):
+def _read_params(repo, configs, rev):
     res = {}
-    for dep in deps:
-        assert dep.scheme == "local"
-
-        params = dep.read_params()
-        if not params:
+    for config in configs:
+        if not repo.tree.exists(fspath_py35(config)):
             continue
 
-        res[str(dep.path_info)] = params
+        with repo.tree.open(fspath_py35(config), "r") as fobj:
+            try:
+                res[str(config)] = yaml.safe_load(fobj)
+            except yaml.YAMLError:
+                logger.debug(
+                    "failed to read '%s' on '%s'", config, rev, exc_info=True
+                )
+                continue
 
     return res
 
@@ -46,8 +50,8 @@ def show(repo, revs=None):
     res = {}
 
     for branch in repo.brancher(revs=revs):
-        entries = _collect_params(repo)
-        params = _read_params(entries)
+        configs = _collect_configs(repo)
+        params = _read_params(repo, configs, branch)
 
         if params:
             res[branch] = params

dvc/utils/diff.py

@@ -15,15 +15,15 @@ def _parse(raw):
         return raw
 
 
-def _diff_vals(old, new):
+def _diff_vals(old, new, with_unchanged):
     if (
         isinstance(new, list)
         and isinstance(old, list)
         and len(old) == len(new) == 1
     ):
-        return _diff_vals(old[0], new[0])
+        return _diff_vals(old[0], new[0], with_unchanged)
 
-    if old == new:
+    if not with_unchanged and old == new:
         return {}
 
     res = {"old": old, "new": new}
@@ -42,7 +42,7 @@ def _flatten(d):
     return defaultdict(lambda: "unable to parse")
 
 
-def _diff_dicts(old_dict, new_dict):
+def _diff_dicts(old_dict, new_dict, with_unchanged):
     new = _flatten(new_dict)
     old = _flatten(old_dict)
 
@@ -53,33 +53,33 @@ def _diff_dicts(old_dict, new_dict):
     for xpath in xpaths:
         old_val = old[xpath]
         new_val = new[xpath]
-        val_diff = _diff_vals(old_val, new_val)
+        val_diff = _diff_vals(old_val, new_val, with_unchanged)
         if val_diff:
             res[xpath] = val_diff
     return dict(res)
 
 
-def _diff(old_raw, new_raw):
+def _diff(old_raw, new_raw, with_unchanged):
     old = _parse(old_raw)
     new = _parse(new_raw)
 
     if isinstance(new, dict) or isinstance(old, dict):
-        return _diff_dicts(old, new)
+        return _diff_dicts(old, new, with_unchanged)
 
-    val_diff = _diff_vals(old, new)
+    val_diff = _diff_vals(old, new, with_unchanged)
     if val_diff:
         return {"": val_diff}
 
     return {}
 
 
-def diff(old, new):
+def diff(old, new, with_unchanged=False):
     paths = set(old.keys())
     paths.update(set(new.keys()))
 
     res = defaultdict(dict)
     for path in paths:
-        path_diff = _diff(old.get(path), new.get(path))
+        path_diff = _diff(old.get(path), new.get(path), with_unchanged)
         if path_diff:
             res[path] = path_diff
     return dict(res)

tests/func/params/test_diff.py

@@ -39,7 +39,7 @@ def test_diff_deleted(tmp_dir, scm, dvc):
     scm.add(["params.yaml", "Dvcfile"])
     scm.commit("bar")
 
-    (tmp_dir / "Dvcfile").unlink()
+    (tmp_dir / "params.yaml").unlink()
 
     assert dvc.params.diff() == {
         "params.yaml": {"foo": {"old": "bar", "new": None}}
@@ -85,3 +85,20 @@ def test_diff_dict(tmp_dir, scm, dvc):
     assert dvc.params.diff() == {
         "params.yaml": {"foo.bar": {"old": "baz", "new": "qux"}}
     }
+
+
+def test_diff_with_unchanged(tmp_dir, scm, dvc):
+    tmp_dir.gen("params.yaml", "foo: bar\nxyz: val")
+    dvc.run(params=["foo,xyz"])
+    scm.add(["params.yaml", "Dvcfile"])
+    scm.commit("bar")
+
+    tmp_dir.scm_gen("params.yaml", "foo: baz\nxyz: val", commit="baz")
+    tmp_dir.scm_gen("params.yaml", "foo: qux\nxyz: val", commit="qux")
+
+    assert dvc.params.diff(a_rev="HEAD~2", all=True) == {
+        "params.yaml": {
+            "foo": {"old": "bar", "new": "qux"},
+            "xyz": {"old": "val", "new": "val"},
+        }
+    }

tests/unit/command/test_params.py

@@ -6,7 +6,7 @@ from dvc.command.params import CmdParamsDiff, _show_diff
 
 def test_params_diff(dvc, mocker):
     cli_args = parse_args(
-        ["params", "diff", "HEAD~10", "HEAD~1", "--show-json"]
+        ["params", "diff", "HEAD~10", "HEAD~1", "--all", "--show-json"]
     )
     assert cli_args.func == CmdParamsDiff
 
@@ -16,7 +16,7 @@ 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",
+        cmd.repo, a_rev="HEAD~10", b_rev="HEAD~1", all=True,
     )