From 542d89dd5499c0bec0af78b4d4669b05df33639e Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Tue, 20 Dec 2022 10:49:49 -0500 Subject: [PATCH 1/4] metrics: fix diff bt active branch and workspace --- dvc/repo/metrics/diff.py | 7 +++++++ tests/func/metrics/test_diff.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/dvc/repo/metrics/diff.py b/dvc/repo/metrics/diff.py index 0f0a2550ae..04ab6c9455 100644 --- a/dvc/repo/metrics/diff.py +++ b/dvc/repo/metrics/diff.py @@ -12,6 +12,13 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): b_rev = b_rev or "workspace" metrics = repo.metrics.show(*args, **kwargs, revs=[a_rev, b_rev]) + + # workspace may have been replaced by active branch + workspace_rev = (a_rev == "workspace") or (b_rev == "workspace") + if workspace_rev and "workspace" not in metrics: + active_branch = repo.scm.active_branch() + metrics["workspace"] = metrics[active_branch] + old = metrics.get(a_rev, {}).get("data", {}) new = metrics.get(b_rev, {}).get("data", {}) diff --git a/tests/func/metrics/test_diff.py b/tests/func/metrics/test_diff.py index 23adde515c..1749b94762 100644 --- a/tests/func/metrics/test_diff.py +++ b/tests/func/metrics/test_diff.py @@ -235,3 +235,20 @@ def test_diff_top_level_metrics(tmp_dir, dvc, scm, dvcfile, metrics_file): "foo": {"diff": 2, "new": 5, "old": 3} } } + + +def test_metrics_diff_active_branch_unchanged( + tmp_dir, scm, dvc, run_copy_metrics +): + def _gen(val): + metrics = {"a": {"b": {"c": val, "d": 1, "e": str(val)}}} + (tmp_dir / "m_temp.yaml").dump(metrics) + run_copy_metrics( + "m_temp.yaml", "m.yaml", metrics=["m.yaml"], commit=str(val) + ) + + _gen(1) + _gen(2) + _gen(1) + + assert dvc.metrics.diff(a_rev=tmp_dir.scm.active_branch()) == {} From 57e27a382a0f3ae696b89e4ed5c112c50afc6821 Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Tue, 20 Dec 2022 12:00:37 -0500 Subject: [PATCH 2/4] params: fix diff bt active branch and workspace --- dvc/repo/params/diff.py | 6 ++++++ tests/func/params/test_diff.py | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/dvc/repo/params/diff.py b/dvc/repo/params/diff.py index 15688852db..46c541a8c7 100644 --- a/dvc/repo/params/diff.py +++ b/dvc/repo/params/diff.py @@ -13,6 +13,12 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): params = repo.params.show(*args, **kwargs, revs=[a_rev, b_rev]) + # workspace may have been replaced by active branch + workspace_rev = (a_rev == "workspace") or (b_rev == "workspace") + if workspace_rev and "workspace" not in params: + active_branch = repo.scm.active_branch() + params["workspace"] = params[active_branch] + old = params.get(a_rev, {}).get("data", {}) new = params.get(b_rev, {}).get("data", {}) diff --git a/tests/func/params/test_diff.py b/tests/func/params/test_diff.py index ba181ba38b..c694e7a43a 100644 --- a/tests/func/params/test_diff.py +++ b/tests/func/params/test_diff.py @@ -268,3 +268,11 @@ def test_diff_top_level_params(tmp_dir, dvc, scm, dvcfile, params_file): "foo": {"diff": 2, "new": 5, "old": 3} } } + + +def test_diff_active_branch_no_changes(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") + assert dvc.params.diff(a_rev=tmp_dir.scm.active_branch()) == {} From f081902e5cf76d66645b5550fdc866476cda93d6 Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Tue, 3 Jan 2023 11:40:35 -0500 Subject: [PATCH 3/4] params/metrics diff: address test failures for missing branch key --- dvc/repo/metrics/diff.py | 3 ++- dvc/repo/params/diff.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dvc/repo/metrics/diff.py b/dvc/repo/metrics/diff.py index 04ab6c9455..f6b8c15164 100644 --- a/dvc/repo/metrics/diff.py +++ b/dvc/repo/metrics/diff.py @@ -17,7 +17,8 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): workspace_rev = (a_rev == "workspace") or (b_rev == "workspace") if workspace_rev and "workspace" not in metrics: active_branch = repo.scm.active_branch() - metrics["workspace"] = metrics[active_branch] + if active_branch in metrics: + metrics["workspace"] = metrics[active_branch] old = metrics.get(a_rev, {}).get("data", {}) new = metrics.get(b_rev, {}).get("data", {}) diff --git a/dvc/repo/params/diff.py b/dvc/repo/params/diff.py index 46c541a8c7..8f87966ae5 100644 --- a/dvc/repo/params/diff.py +++ b/dvc/repo/params/diff.py @@ -17,7 +17,8 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): workspace_rev = (a_rev == "workspace") or (b_rev == "workspace") if workspace_rev and "workspace" not in params: active_branch = repo.scm.active_branch() - params["workspace"] = params[active_branch] + if active_branch in params: + params["workspace"] = params[active_branch] old = params.get(a_rev, {}).get("data", {}) new = params.get(b_rev, {}).get("data", {}) From ed689da3e7d8713b6cea9d881de537d620b0e28f Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Mon, 9 Jan 2023 16:20:26 +0200 Subject: [PATCH 4/4] metrics/params: show: add hide_workspace flag --- dvc/repo/metrics/diff.py | 11 +++-------- dvc/repo/metrics/show.py | 22 ++++++++++++---------- dvc/repo/params/diff.py | 11 +++-------- dvc/repo/params/show.py | 22 ++++++++++++---------- 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/dvc/repo/metrics/diff.py b/dvc/repo/metrics/diff.py index f6b8c15164..abede5c8a4 100644 --- a/dvc/repo/metrics/diff.py +++ b/dvc/repo/metrics/diff.py @@ -11,14 +11,9 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): a_rev = a_rev or "HEAD" b_rev = b_rev or "workspace" - metrics = repo.metrics.show(*args, **kwargs, revs=[a_rev, b_rev]) - - # workspace may have been replaced by active branch - workspace_rev = (a_rev == "workspace") or (b_rev == "workspace") - if workspace_rev and "workspace" not in metrics: - active_branch = repo.scm.active_branch() - if active_branch in metrics: - metrics["workspace"] = metrics[active_branch] + metrics = repo.metrics.show( + *args, **kwargs, revs=[a_rev, b_rev], hide_workspace=False + ) old = metrics.get(a_rev, {}).get("data", {}) new = metrics.get(b_rev, {}).get("data", {}) diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 82ca5cfa4c..d482b2db01 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -124,6 +124,7 @@ def show( revs=None, all_commits=False, onerror=None, + hide_workspace=True, ): if onerror is None: onerror = onerror_collect @@ -142,16 +143,17 @@ def show( repo, targets, rev, recursive, onerror=onerror ) - # Hide workspace metrics if they are the same as in the active branch - try: - active_branch = repo.scm.active_branch() - except (SCMError, NoSCMError): - # SCMError - detached head - # NoSCMError - no repo case - pass - else: - if res.get("workspace") == res.get(active_branch): - res.pop("workspace", None) + if hide_workspace: + # Hide workspace metrics if they are the same as in the active branch + try: + active_branch = repo.scm.active_branch() + except (SCMError, NoSCMError): + # SCMError - detached head + # NoSCMError - no repo case + pass + else: + if res.get("workspace") == res.get(active_branch): + res.pop("workspace", None) errored = errored_revisions(res) if errored: diff --git a/dvc/repo/params/diff.py b/dvc/repo/params/diff.py index 8f87966ae5..79c68b1e4e 100644 --- a/dvc/repo/params/diff.py +++ b/dvc/repo/params/diff.py @@ -11,14 +11,9 @@ def diff(repo, *args, a_rev=None, b_rev=None, **kwargs): a_rev = a_rev or "HEAD" b_rev = b_rev or "workspace" - params = repo.params.show(*args, **kwargs, revs=[a_rev, b_rev]) - - # workspace may have been replaced by active branch - workspace_rev = (a_rev == "workspace") or (b_rev == "workspace") - if workspace_rev and "workspace" not in params: - active_branch = repo.scm.active_branch() - if active_branch in params: - params["workspace"] = params[active_branch] + params = repo.params.show( + *args, **kwargs, revs=[a_rev, b_rev], hide_workspace=False + ) old = params.get(a_rev, {}).get("data", {}) new = params.get(b_rev, {}).get("data", {}) diff --git a/dvc/repo/params/show.py b/dvc/repo/params/show.py index f4a9b48c32..c432f89d84 100644 --- a/dvc/repo/params/show.py +++ b/dvc/repo/params/show.py @@ -144,6 +144,7 @@ def show( deps=False, onerror: Callable = None, stages=None, + hide_workspace=True, ): if onerror is None: onerror = onerror_collect @@ -165,16 +166,17 @@ def show( if params: res[branch] = params - # Hide workspace params if they are the same as in the active branch - try: - active_branch = repo.scm.active_branch() - except (SCMError, NoSCMError): - # SCMError - detached head - # NoSCMError - no repo case - pass - else: - if res.get("workspace") == res.get(active_branch): - res.pop("workspace", None) + if hide_workspace: + # Hide workspace params if they are the same as in the active branch + try: + active_branch = repo.scm.active_branch() + except (SCMError, NoSCMError): + # SCMError - detached head + # NoSCMError - no repo case + pass + else: + if res.get("workspace") == res.get(active_branch): + res.pop("workspace", None) errored = errored_revisions(res) if errored: