diff --git a/dvc/command/experiments.py b/dvc/command/experiments.py index f53a4d080..d64f9eb1f 100644 --- a/dvc/command/experiments.py +++ b/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="", ) + 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", diff --git a/dvc/command/params.py b/dvc/command/params.py index 717be814c..600981070 100644 --- a/dvc/command/params.py +++ b/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", diff --git a/dvc/dependency/param.py b/dvc/dependency/param.py index 5d015f1ae..cb0dad2da 100644 --- a/dvc/dependency/param.py +++ b/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: diff --git a/dvc/repo/experiments/diff.py b/dvc/repo/experiments/diff.py index b03f6ba95..7864056e3 100644 --- a/dvc/repo/experiments/diff.py +++ b/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) diff --git a/dvc/repo/experiments/show.py b/dvc/repo/experiments/show.py index 39ac7e66d..f4c90eaf3 100644 --- a/dvc/repo/experiments/show.py +++ b/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 diff --git a/dvc/repo/params/show.py b/dvc/repo/params/show.py index 7ca660302..0e1138d0a 100644 --- a/dvc/repo/params/show.py +++ b/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 diff --git a/tests/func/params/test_diff.py b/tests/func/params/test_diff.py index ba8946778..10377bc8d 100644 --- a/tests/func/params/test_diff.py +++ b/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) diff --git a/tests/func/params/test_show.py b/tests/func/params/test_show.py index a9a897677..d89e889aa 100644 --- a/tests/func/params/test_show.py +++ b/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): diff --git a/tests/unit/command/test_experiments.py b/tests/unit/command/test_experiments.py index 65e50634e..af6805a2a 100644 --- a/tests/unit/command/test_experiments.py +++ b/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, ) diff --git a/tests/unit/command/test_params.py b/tests/unit/command/test_params.py index 15e0f5099..700c25b19 100644 --- a/tests/unit/command/test_params.py +++ b/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, ) diff --git a/tests/unit/test_params.py b/tests/unit/test_params.py index 44b38a4f8..2b7d3dd51 100644 --- a/tests/unit/test_params.py +++ b/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):