diff --git a/dvc/command/params.py b/dvc/command/params.py index 52dad57b69..0281228df9 100644 --- a/dvc/command/params.py +++ b/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", diff --git a/dvc/repo/params/diff.py b/dvc/repo/params/diff.py index 753fec5e41..0b22e212c0 100644 --- a/dvc/repo/params/diff.py +++ b/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 + ) diff --git a/dvc/repo/params/show.py b/dvc/repo/params/show.py index 1391154331..b619a8fec1 100644 --- a/dvc/repo/params/show.py +++ b/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) + configs.add(dep.path_info) + return list(configs) - return configs.values() - -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 diff --git a/dvc/utils/diff.py b/dvc/utils/diff.py index 35973491af..0341e1937c 100644 --- a/dvc/utils/diff.py +++ b/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) diff --git a/tests/func/params/test_diff.py b/tests/func/params/test_diff.py index 1645c46e8b..0f8dc31a86 100644 --- a/tests/func/params/test_diff.py +++ b/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"}, + } + } diff --git a/tests/unit/command/test_params.py b/tests/unit/command/test_params.py index 16df62d90b..bb0b6f1368 100644 --- a/tests/unit/command/test_params.py +++ b/tests/unit/command/test_params.py @@ -6,7 +6,7 @@ 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, )