From b54c8516a15683f92966c3a8c371e41bb82dfd08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Mon, 23 Nov 2020 17:30:55 +0100 Subject: [PATCH 1/8] params: add targets option --- dvc/command/params.py | 8 +++++++ dvc/repo/params/show.py | 30 +++++++++++++++++-------- tests/func/metrics/metrics/test_show.py | 5 ++++- tests/func/params/test_show.py | 11 +++++++++ tests/unit/command/test_params.py | 8 ++++++- 5 files changed, 51 insertions(+), 11 deletions(-) diff --git a/dvc/command/params.py b/dvc/command/params.py index f980ff69b6..b91b1ac321 100644 --- a/dvc/command/params.py +++ b/dvc/command/params.py @@ -2,6 +2,7 @@ import logging from collections import OrderedDict +from dvc.command import completion from dvc.command.base import CmdBase, append_doc_link, fix_subparsers from dvc.exceptions import DvcException @@ -35,6 +36,7 @@ def run(self): diff = self.repo.params.diff( a_rev=self.args.a_rev, b_rev=self.args.b_rev, + targets=self.args.targets, all=self.args.all, ) @@ -91,6 +93,12 @@ def add_parser(subparsers, parent_parser): nargs="?", help=("New Git commit to compare (defaults to the current workspace)"), ) + params_diff_parser.add_argument( + "--targets", + nargs="*", + help="Limit command scope to these params files.", + metavar="", + ).complete = completion.FILE params_diff_parser.add_argument( "--all", action="store_true", diff --git a/dvc/repo/params/show.py b/dvc/repo/params/show.py index e6199d185d..c2487e86e9 100644 --- a/dvc/repo/params/show.py +++ b/dvc/repo/params/show.py @@ -8,6 +8,7 @@ from dvc.repo import locked from dvc.repo.collect import collect from dvc.stage import PipelineStage +from dvc.scm.base import SCMError from dvc.utils.serialize import LOADERS, ParseError if TYPE_CHECKING: @@ -25,11 +26,20 @@ def _is_params(dep: "BaseOutput"): return isinstance(dep, ParamsDependency) -def _collect_configs(repo: "Repo", rev) -> List[PathInfo]: - params, _ = collect(repo, deps=True, output_filter=_is_params, rev=rev) - configs = {p.path_info for p in params} - configs.add(PathInfo(repo.root_dir) / ParamsDependency.DEFAULT_PARAMS_FILE) - return list(configs) +def _collect_configs(repo: "Repo", rev, targets=None) -> List[PathInfo]: + params, path_infos = collect( + repo, + targets=targets or [], + deps=True, + output_filter=_is_params, + rev=rev, + ) + path_infos.update({p.path_info for p in params}) + if not targets: + path_infos.add( + PathInfo(repo.root_dir) / ParamsDependency.DEFAULT_PARAMS_FILE + ) + return list(path_infos) def _read_params(repo, configs, rev): @@ -66,11 +76,11 @@ def _collect_vars(repo, params): @locked -def show(repo, revs=None): +def show(repo, revs=None, targets=None): res = {} for branch in repo.brancher(revs=revs): - configs = _collect_configs(repo, branch) + configs = _collect_configs(repo, branch, targets) params = _read_params(repo, configs, branch) vars_params = _collect_vars(repo, params) @@ -87,8 +97,10 @@ def show(repo, revs=None): # Hide workspace params if they are the same as in the active branch try: active_branch = repo.scm.active_branch() - except TypeError: - pass # Detached head + except (TypeError, SCMError): + # TypeError - detached head + # SCMError - no repo case + pass else: if res.get("workspace") == res.get(active_branch): res.pop("workspace", None) diff --git a/tests/func/metrics/metrics/test_show.py b/tests/func/metrics/metrics/test_show.py index 7f78f0a40d..a29cae2260 100644 --- a/tests/func/metrics/metrics/test_show.py +++ b/tests/func/metrics/metrics/test_show.py @@ -161,8 +161,11 @@ def test_show_falsey(tmp_dir, dvc): def test_show_no_repo(tmp_dir): - tmp_dir.gen("metrics.json", '{"foo": 0, "bar": 0.0, "baz": {}}') + tmp_dir.gen("metrics.json", '{"foo": 0, "bar": 0.0}') dvc = Repo(uninitialized=True) dvc.metrics.show(targets=["metrics.json"]) + assert dvc.metrics.show(targets=["metrics.json"]) == { + "": {"metrics.json": {"foo": 0, "bar": 0.0}} + } diff --git a/tests/func/params/test_show.py b/tests/func/params/test_show.py index cdd39f5e31..a9a897677d 100644 --- a/tests/func/params/test_show.py +++ b/tests/func/params/test_show.py @@ -1,5 +1,6 @@ import pytest +from dvc.repo import Repo from dvc.repo.params.show import NoParamsError @@ -93,3 +94,13 @@ def test_pipeline_tracked_params(tmp_dir, scm, dvc, run_copy): assert dvc.params.show(revs=["master"]) == { "master": {"params.yaml": {"foo": "qux", "xyz": "val"}} } + + +def test_show_no_repo(tmp_dir): + tmp_dir.gen({"foo": "foo", "params_file.yaml": "foo: bar\nxyz: val"}) + + dvc = Repo(uninitialized=True) + + assert dvc.params.show(targets=["params_file.yaml"]) == { + "": {"params_file.yaml": {"foo": "bar", "xyz": "val"}} + } diff --git a/tests/unit/command/test_params.py b/tests/unit/command/test_params.py index 4c61c60ec2..15e0f5099b 100644 --- a/tests/unit/command/test_params.py +++ b/tests/unit/command/test_params.py @@ -12,6 +12,8 @@ def test_params_diff(dvc, mocker): "diff", "HEAD~10", "HEAD~1", + "--targets", + "target", "--all", "--show-json", "--show-md", @@ -26,7 +28,11 @@ 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", all=True, + cmd.repo, + a_rev="HEAD~10", + b_rev="HEAD~1", + targets=["target"], + all=True, ) From 945dc4e14e6830a097c057623e88e9094fb31789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Mon, 23 Nov 2020 19:20:04 +0100 Subject: [PATCH 2/8] fixup --- dvc/command/params.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dvc/command/params.py b/dvc/command/params.py index b91b1ac321..88a41204d3 100644 --- a/dvc/command/params.py +++ b/dvc/command/params.py @@ -31,6 +31,8 @@ def _show_diff(diff, markdown=False, no_path=False): class CmdParamsDiff(CmdBase): + UNINITIALIZED = True + def run(self): try: diff = self.repo.params.diff( From 81366a9189c77bf980344da63f9be0548c66b223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Wed, 16 Dec 2020 23:00:09 +0100 Subject: [PATCH 3/8] remove unnecessary changes --- tests/func/metrics/metrics/test_show.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/func/metrics/metrics/test_show.py b/tests/func/metrics/metrics/test_show.py index a29cae2260..7f78f0a40d 100644 --- a/tests/func/metrics/metrics/test_show.py +++ b/tests/func/metrics/metrics/test_show.py @@ -161,11 +161,8 @@ def test_show_falsey(tmp_dir, dvc): def test_show_no_repo(tmp_dir): - tmp_dir.gen("metrics.json", '{"foo": 0, "bar": 0.0}') + tmp_dir.gen("metrics.json", '{"foo": 0, "bar": 0.0, "baz": {}}') dvc = Repo(uninitialized=True) dvc.metrics.show(targets=["metrics.json"]) - assert dvc.metrics.show(targets=["metrics.json"]) == { - "": {"metrics.json": {"foo": 0, "bar": 0.0}} - } From 1d22c6f6c09d36aaf570f95d93623d96ac2ecde9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Thu, 17 Dec 2020 08:03:35 +0100 Subject: [PATCH 4/8] fix isort --- dvc/repo/params/show.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/repo/params/show.py b/dvc/repo/params/show.py index c2487e86e9..e0084488a1 100644 --- a/dvc/repo/params/show.py +++ b/dvc/repo/params/show.py @@ -7,8 +7,8 @@ from dvc.path_info import PathInfo from dvc.repo import locked from dvc.repo.collect import collect -from dvc.stage import PipelineStage from dvc.scm.base import SCMError +from dvc.stage import PipelineStage from dvc.utils.serialize import LOADERS, ParseError if TYPE_CHECKING: From 9ac08c61da1c056c2d4e30d97e2eaabc4087b609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Thu, 17 Dec 2020 14:27:55 +0100 Subject: [PATCH 5/8] params: diff: add test for --targets --- tests/func/params/test_diff.py | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/func/params/test_diff.py b/tests/func/params/test_diff.py index 80771079f8..ba89467785 100644 --- a/tests/func/params/test_diff.py +++ b/tests/func/params/test_diff.py @@ -189,3 +189,46 @@ def test_vars_shows_on_params_diff(tmp_dir, scm, dvc): "vars.model1.epoch": {"new": 20, "old": 15, "diff": 5}, } } + + +def test_diff_targeted(tmp_dir, scm, dvc, run_copy): + from dvc.dvcfile import PIPELINE_FILE + + tmp_dir.gen( + { + "foo": "foo", + "params.yaml": "foo: bar", + "other_params.yaml": "xyz: val", + } + ) + run_copy( + "foo", + "bar", + name="copy-foo-bar", + params=["foo", "other_params.yaml:xyz"], + ) + + scm.add(["params.yaml", "other_params.yaml", PIPELINE_FILE]) + scm.commit("add stage") + + tmp_dir.scm_gen( + {"params.yaml": "foo: baz", "other_params.yaml": "xyz: val2"}, + commit="baz", + ) + tmp_dir.scm_gen( + {"params.yaml": "foo: qux", "other_params.yaml": "xyz: val3"}, + commit="qux", + ) + + assert dvc.params.diff(a_rev="HEAD~2") == { + "params.yaml": {"foo": {"old": "bar", "new": "qux"}}, + "other_params.yaml": {"xyz": {"old": "val", "new": "val3"}}, + } + + assert dvc.params.diff(a_rev="HEAD~2", targets=["params.yaml"]) == { + "params.yaml": {"foo": {"old": "bar", "new": "qux"}} + } + + assert dvc.params.diff(a_rev="HEAD~2", targets=["other_params.yaml"]) == { + "other_params.yaml": {"xyz": {"old": "val", "new": "val3"}}, + } From 650035d59b4d1ad5294b2005d2b5c96cfe8587b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Thu, 17 Dec 2020 17:21:15 +0100 Subject: [PATCH 6/8] tests: roll back metric/plots unification, introduce collect test --- tests/func/metrics/plots/__init__.py | 0 tests/func/metrics/test_common.py | 108 ------------------ tests/func/metrics/{metrics => }/test_diff.py | 0 tests/func/metrics/{metrics => }/test_show.py | 54 +++++++++ .../{metrics/metrics => plots}/__init__.py | 0 tests/func/{metrics => }/plots/conftest.py | 0 tests/func/{metrics => }/plots/test_diff.py | 2 +- tests/func/{metrics => }/plots/test_modify.py | 2 +- tests/func/{metrics => }/plots/test_show.py | 48 +++++++- tests/func/{metrics => plots}/utils.py | 0 tests/unit/test_collect.py | 10 ++ 11 files changed, 112 insertions(+), 112 deletions(-) delete mode 100644 tests/func/metrics/plots/__init__.py delete mode 100644 tests/func/metrics/test_common.py rename tests/func/metrics/{metrics => }/test_diff.py (100%) rename tests/func/metrics/{metrics => }/test_show.py (76%) rename tests/func/{metrics/metrics => plots}/__init__.py (100%) rename tests/func/{metrics => }/plots/conftest.py (100%) rename tests/func/{metrics => }/plots/test_diff.py (97%) rename tests/func/{metrics => }/plots/test_modify.py (97%) rename tests/func/{metrics => }/plots/test_show.py (93%) rename tests/func/{metrics => plots}/utils.py (100%) create mode 100644 tests/unit/test_collect.py diff --git a/tests/func/metrics/plots/__init__.py b/tests/func/metrics/plots/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/func/metrics/test_common.py b/tests/func/metrics/test_common.py deleted file mode 100644 index 1a3d3735c8..0000000000 --- a/tests/func/metrics/test_common.py +++ /dev/null @@ -1,108 +0,0 @@ -import logging - -import pytest - -from dvc.exceptions import ( - NoMetricsFoundError, - NoMetricsParsedError, - OverlappingOutputPathsError, -) -from dvc.path_info import PathInfo -from dvc.utils.fs import remove -from dvc.utils.serialize import dump_yaml, modify_yaml -from tests.func.metrics.utils import _write_json - - -@pytest.mark.parametrize( - "diff, metric_value", - ( - ( - lambda repo, target, rev: repo.metrics.diff( - targets=[target], a_rev=rev - ), - {"m": 1}, - ), - ( - lambda repo, target, rev: repo.plots.diff( - targets=[target], revs=[rev] - ), - [{"m": 1}, {"m": 2}], - ), - ), -) -def test_diff_no_file_on_target_rev( - tmp_dir, scm, dvc, caplog, diff, metric_value -): - with tmp_dir.branch("new_branch", new=True): - _write_json(tmp_dir, metric_value, "metric.json") - - with caplog.at_level(logging.WARNING, "dvc"): - diff(dvc, "metric.json", "master") - - assert "'metric.json' was not found at: 'master'." in caplog.text - - -@pytest.mark.parametrize( - "show, malformed_metric", - ( - (lambda repo, target: repo.metrics.show(targets=[target]), '{"m": 1'), - ( - lambda repo, target: repo.plots.show(targets=[target]), - '[{"m": 1}, {"m": 2}', - ), - ), -) -def test_show_malformed_metric( - tmp_dir, scm, dvc, caplog, show, malformed_metric -): - tmp_dir.gen("metric.json", malformed_metric) - - with pytest.raises(NoMetricsParsedError): - show(dvc, "metric.json") - - -@pytest.mark.parametrize( - "show", - (lambda repo: repo.metrics.show(), lambda repo: repo.plots.show(),), -) -def test_show_no_metrics_files(tmp_dir, dvc, caplog, show): - with pytest.raises(NoMetricsFoundError): - show(dvc) - - -@pytest.mark.parametrize("clear_before_run", [True, False]) -@pytest.mark.parametrize("typ", ["metrics", "plots"]) -def test_metrics_show_overlap( - tmp_dir, dvc, run_copy_metrics, clear_before_run, typ -): - data_dir = PathInfo("data") - (tmp_dir / data_dir).mkdir() - - outs = {typ: [str(data_dir / "m1.yaml")]} - dump_yaml(data_dir / "m1_temp.yaml", {"a": {"b": {"c": 2, "d": 1}}}) - run_copy_metrics( - str(data_dir / "m1_temp.yaml"), - str(data_dir / "m1.yaml"), - single_stage=False, - commit=f"add m1 {typ}", - name="cp-m1", - **outs, - ) - with modify_yaml("dvc.yaml") as d: - # trying to make an output overlaps error - d["stages"]["corrupted-stage"] = { - "cmd": "mkdir data", - "outs": ["data"], - } - - # running by clearing and not clearing stuffs - # so as it works even for optimized cases - if clear_before_run: - remove(data_dir) - remove(dvc.cache.local.cache_dir) - - dvc._reset() - - show = dvc.metrics.show if typ == "metrics" else dvc.plots.show - with pytest.raises(OverlappingOutputPathsError): - show() diff --git a/tests/func/metrics/metrics/test_diff.py b/tests/func/metrics/test_diff.py similarity index 100% rename from tests/func/metrics/metrics/test_diff.py rename to tests/func/metrics/test_diff.py diff --git a/tests/func/metrics/metrics/test_show.py b/tests/func/metrics/test_show.py similarity index 76% rename from tests/func/metrics/metrics/test_show.py rename to tests/func/metrics/test_show.py index 7f78f0a40d..28960293a1 100644 --- a/tests/func/metrics/metrics/test_show.py +++ b/tests/func/metrics/test_show.py @@ -2,8 +2,15 @@ import pytest +from dvc.exceptions import ( + NoMetricsFoundError, + NoMetricsParsedError, + OverlappingOutputPathsError, +) +from dvc.path_info import PathInfo from dvc.repo import Repo from dvc.utils.fs import remove +from dvc.utils.serialize import dump_yaml, modify_yaml def test_show_simple(tmp_dir, dvc, run_copy_metrics): @@ -166,3 +173,50 @@ def test_show_no_repo(tmp_dir): dvc = Repo(uninitialized=True) dvc.metrics.show(targets=["metrics.json"]) + + +def test_show_malformed_metric(tmp_dir, scm, dvc, caplog): + tmp_dir.gen("metric.json", '{"m":1') + + with pytest.raises(NoMetricsParsedError): + dvc.metrics.show(targets=["metric.json"]) + + +def test_show_no_metrics_files(tmp_dir, dvc, caplog): + with pytest.raises(NoMetricsFoundError): + dvc.metrics.show() + + +@pytest.mark.parametrize("clear_before_run", [True, False]) +def test_metrics_show_overlap( + tmp_dir, dvc, run_copy_metrics, clear_before_run +): + data_dir = PathInfo("data") + (tmp_dir / data_dir).mkdir() + + dump_yaml(data_dir / "m1_temp.yaml", {"a": {"b": {"c": 2, "d": 1}}}) + run_copy_metrics( + str(data_dir / "m1_temp.yaml"), + str(data_dir / "m1.yaml"), + single_stage=False, + commit="add m1", + name="cp-m1", + metrics=[str(data_dir / "m1.yaml")], + ) + with modify_yaml("dvc.yaml") as d: + # trying to make an output overlaps error + d["stages"]["corrupted-stage"] = { + "cmd": "mkdir data", + "outs": ["data"], + } + + # running by clearing and not clearing stuffs + # so as it works even for optimized cases + if clear_before_run: + remove(data_dir) + remove(dvc.cache.local.cache_dir) + + dvc._reset() + + with pytest.raises(OverlappingOutputPathsError): + dvc.metrics.show() diff --git a/tests/func/metrics/metrics/__init__.py b/tests/func/plots/__init__.py similarity index 100% rename from tests/func/metrics/metrics/__init__.py rename to tests/func/plots/__init__.py diff --git a/tests/func/metrics/plots/conftest.py b/tests/func/plots/conftest.py similarity index 100% rename from tests/func/metrics/plots/conftest.py rename to tests/func/plots/conftest.py diff --git a/tests/func/metrics/plots/test_diff.py b/tests/func/plots/test_diff.py similarity index 97% rename from tests/func/metrics/plots/test_diff.py rename to tests/func/plots/test_diff.py index b401011e7e..7162b88ac6 100644 --- a/tests/func/metrics/plots/test_diff.py +++ b/tests/func/plots/test_diff.py @@ -1,7 +1,7 @@ import json from dvc.repo.plots.data import PlotData -from tests.func.metrics.utils import _write_json +from tests.func.plots.utils import _write_json def test_diff_dirty(tmp_dir, scm, dvc, run_copy_metrics): diff --git a/tests/func/metrics/plots/test_modify.py b/tests/func/plots/test_modify.py similarity index 97% rename from tests/func/metrics/plots/test_modify.py rename to tests/func/plots/test_modify.py index c8bf354b42..3cc2fc9513 100644 --- a/tests/func/metrics/plots/test_modify.py +++ b/tests/func/plots/test_modify.py @@ -4,7 +4,7 @@ from dvc.repo.plots import PropsNotFoundError from dvc.repo.plots.template import TemplateNotFoundError from dvc.utils import relpath -from tests.func.metrics.utils import _write_json +from tests.func.plots.utils import _write_json def test_plots_modify_existing_template( diff --git a/tests/func/metrics/plots/test_show.py b/tests/func/plots/test_show.py similarity index 93% rename from tests/func/metrics/plots/test_show.py rename to tests/func/plots/test_show.py index 544867a396..ab25aeedcd 100644 --- a/tests/func/metrics/plots/test_show.py +++ b/tests/func/plots/test_show.py @@ -6,7 +6,9 @@ import pytest +from dvc.exceptions import NoMetricsParsedError, OverlappingOutputPathsError from dvc.main import main +from dvc.path_info import PathInfo from dvc.repo import Repo from dvc.repo.plots.data import ( JSONPlotData, @@ -21,8 +23,8 @@ TemplateNotFoundError, ) from dvc.utils.fs import remove -from dvc.utils.serialize import dump_yaml, dumps_yaml -from tests.func.metrics.utils import _write_csv, _write_json +from dvc.utils.serialize import dump_yaml, dumps_yaml, modify_yaml +from tests.func.plots.utils import _write_csv, _write_json def test_plot_csv_one_column(tmp_dir, scm, dvc, run_copy_metrics): @@ -704,3 +706,45 @@ def test_show_from_subdir(tmp_dir, dvc, caplog): assert f"file://{str(subdir)}" in caplog.text assert (subdir / "plots.html").exists() + + +def test_show_malformed_plot(tmp_dir, scm, dvc, caplog): + tmp_dir.gen("plot.json", '[{"m":1]') + + with pytest.raises(NoMetricsParsedError): + dvc.metrics.show(targets=["plot.json"]) + + +@pytest.mark.parametrize("clear_before_run", [True, False]) +def test_metrics_show_overlap( + tmp_dir, dvc, run_copy_metrics, clear_before_run +): + data_dir = PathInfo("data") + (tmp_dir / data_dir).mkdir() + + dump_yaml(data_dir / "m1_temp.yaml", {"a": {"b": {"c": 2, "d": 1}}}) + run_copy_metrics( + str(data_dir / "m1_temp.yaml"), + str(data_dir / "m1.yaml"), + single_stage=False, + commit="add m1", + name="cp-m1", + plots=[str(data_dir / "m1.yaml")], + ) + with modify_yaml("dvc.yaml") as d: + # trying to make an output overlaps error + d["stages"]["corrupted-stage"] = { + "cmd": "mkdir data", + "outs": ["data"], + } + + # running by clearing and not clearing stuffs + # so as it works even for optimized cases + if clear_before_run: + remove(data_dir) + remove(dvc.cache.local.cache_dir) + + dvc._reset() + + with pytest.raises(OverlappingOutputPathsError): + dvc.plots.show() diff --git a/tests/func/metrics/utils.py b/tests/func/plots/utils.py similarity index 100% rename from tests/func/metrics/utils.py rename to tests/func/plots/utils.py diff --git a/tests/unit/test_collect.py b/tests/unit/test_collect.py new file mode 100644 index 0000000000..8b709d1d9a --- /dev/null +++ b/tests/unit/test_collect.py @@ -0,0 +1,10 @@ +import logging + +from dvc.repo.collect import collect + + +def test_diff_no_file_on_target_rev(tmp_dir, scm, dvc, caplog): + with caplog.at_level(logging.WARNING, "dvc"): + collect(dvc, targets=["file.yaml"], rev="current_branch") + + assert "'file.yaml' was not found at: 'current_branch'." in caplog.text From abc8387f319417cc39a6844cd152327551933ad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Thu, 17 Dec 2020 22:50:54 +0100 Subject: [PATCH 7/8] metrics/plots: refactoring --- dvc/exceptions.py | 17 ++++++++++++++--- dvc/repo/metrics/diff.py | 4 ++-- dvc/repo/metrics/show.py | 8 +++++++- dvc/repo/plots/__init__.py | 4 ++-- dvc/repo/plots/data.py | 5 ----- tests/func/plots/test_show.py | 13 ++++++++----- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 7e6a5745e5..3d182233cd 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -1,4 +1,6 @@ """Exceptions raised by the dvc.""" +from typing import List + from funcy import first from dvc.utils import error_link, format_link, relpath @@ -170,11 +172,11 @@ def __init__(self, paths): ) -class NoMetricsError(DvcException): +class MetricsError(DvcException): pass -class NoMetricsParsedError(NoMetricsError): +class NoMetricsParsedError(MetricsError): def __init__(self, command): super().__init__( f"Could not parse {command} files. Use `-v` option to see more " @@ -182,7 +184,7 @@ def __init__(self, command): ) -class NoMetricsFoundError(NoMetricsError): +class NoMetricsFoundError(MetricsError): def __init__(self, command, run_options): super().__init__( f"No {command} files in this repository. " @@ -191,6 +193,15 @@ def __init__(self, command, run_options): ) +class MetricDoesNotExistError(MetricsError): + def __init__(self, targets: List[str]): + if len(targets) == 1: + msg = "File: '{}' does not exist." + else: + msg = "Files: '{}' do not exist." + super().__init__(msg.format(", ".join(targets))) + + class RecursiveAddingWhileUsingFilename(DvcException): def __init__(self): super().__init__( diff --git a/dvc/repo/metrics/diff.py b/dvc/repo/metrics/diff.py index 329a24272d..c49f867559 100644 --- a/dvc/repo/metrics/diff.py +++ b/dvc/repo/metrics/diff.py @@ -1,4 +1,4 @@ -from dvc.exceptions import NoMetricsError +from dvc.exceptions import MetricsError from dvc.utils.diff import diff as _diff from dvc.utils.diff import format_dict @@ -7,7 +7,7 @@ def _get_metrics(repo, *args, revs=None, **kwargs): try: metrics = repo.metrics.show(*args, **kwargs, revs=revs) return metrics - except NoMetricsError: + except MetricsError: return {} diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 6a08224c45..2f5c8c9869 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -1,6 +1,10 @@ import logging -from dvc.exceptions import NoMetricsFoundError, NoMetricsParsedError +from dvc.exceptions import ( + MetricDoesNotExistError, + NoMetricsFoundError, + NoMetricsParsedError, +) from dvc.repo import locked from dvc.repo.collect import collect from dvc.scm.base import SCMError @@ -105,6 +109,8 @@ def show( if not res: if metrics_found: raise NoMetricsParsedError("metrics") + elif targets: + raise MetricDoesNotExistError(targets) else: raise NoMetricsFoundError("metrics", "-m/-M") diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index e4ad6fa390..fa030c177e 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -4,6 +4,7 @@ from dvc.exceptions import ( DvcException, + MetricDoesNotExistError, NoMetricsFoundError, NoMetricsParsedError, ) @@ -96,7 +97,6 @@ def render(data, revs=None, props=None, templates=None): return result def show(self, targets=None, revs=None, props=None, templates=None): - from .data import NoMetricInHistoryError data = self.collect(targets, revs) @@ -105,7 +105,7 @@ def show(self, targets=None, revs=None, props=None, templates=None): for target in targets: rpath = relpath(target, self.repo.root_dir) if not any("data" in d[rpath] for d in data.values()): - raise NoMetricInHistoryError(target) + raise MetricDoesNotExistError([target]) # No data at all is a special error with a special message if not data: diff --git a/dvc/repo/plots/data.py b/dvc/repo/plots/data.py index bff7aeb3d4..4434b73660 100644 --- a/dvc/repo/plots/data.py +++ b/dvc/repo/plots/data.py @@ -26,11 +26,6 @@ def __init__(self): ) -class NoMetricInHistoryError(DvcException): - def __init__(self, path): - super().__init__(f"Could not find '{path}'.") - - class PlotParsingError(ParseError): def __init__(self, path, revision): self.path = path diff --git a/tests/func/plots/test_show.py b/tests/func/plots/test_show.py index ab25aeedcd..9a74aa51d8 100644 --- a/tests/func/plots/test_show.py +++ b/tests/func/plots/test_show.py @@ -6,13 +6,16 @@ import pytest -from dvc.exceptions import NoMetricsParsedError, OverlappingOutputPathsError +from dvc.exceptions import ( + MetricDoesNotExistError, + NoMetricsParsedError, + OverlappingOutputPathsError, +) from dvc.main import main from dvc.path_info import PathInfo from dvc.repo import Repo from dvc.repo.plots.data import ( JSONPlotData, - NoMetricInHistoryError, PlotData, PlotMetricTypeError, YAMLPlotData, @@ -401,15 +404,15 @@ def test_throw_on_no_metric_at_all(tmp_dir, scm, dvc, caplog): tmp_dir.gen("some_file", "make repo dirty") caplog.clear() - with pytest.raises(NoMetricInHistoryError) as error, caplog.at_level( + with pytest.raises(MetricDoesNotExistError) as error, caplog.at_level( logging.WARNING, "dvc" ): - dvc.plots.show(targets="metric.json", revs=["v1"]) + dvc.plots.show(targets="plot.json", revs=["v1"]) # do not warn if none found assert len(caplog.messages) == 0 - assert str(error.value) == "Could not find 'metric.json'." + assert str(error.value) == "File: 'plot.json' does not exist." def test_custom_template(tmp_dir, scm, dvc, custom_template, run_copy_metrics): From f1f5060280f2683977ac1f7bb0e11fa4b08e69aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Fri, 18 Dec 2020 12:20:28 +0100 Subject: [PATCH 8/8] missing plots test added --- tests/func/metrics/test_show.py | 6 ++++++ tests/func/plots/test_show.py | 17 +++++++++++++---- tests/unit/test_collect.py | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tests/func/metrics/test_show.py b/tests/func/metrics/test_show.py index 28960293a1..64fcf156ca 100644 --- a/tests/func/metrics/test_show.py +++ b/tests/func/metrics/test_show.py @@ -3,6 +3,7 @@ import pytest from dvc.exceptions import ( + MetricDoesNotExistError, NoMetricsFoundError, NoMetricsParsedError, OverlappingOutputPathsError, @@ -182,6 +183,11 @@ def test_show_malformed_metric(tmp_dir, scm, dvc, caplog): dvc.metrics.show(targets=["metric.json"]) +def test_metrics_show_no_target(tmp_dir, dvc): + with pytest.raises(MetricDoesNotExistError): + dvc.metrics.show(targets=["metrics.json"]) + + def test_show_no_metrics_files(tmp_dir, dvc, caplog): with pytest.raises(NoMetricsFoundError): dvc.metrics.show() diff --git a/tests/func/plots/test_show.py b/tests/func/plots/test_show.py index 9a74aa51d8..da2816ec46 100644 --- a/tests/func/plots/test_show.py +++ b/tests/func/plots/test_show.py @@ -8,6 +8,7 @@ from dvc.exceptions import ( MetricDoesNotExistError, + NoMetricsFoundError, NoMetricsParsedError, OverlappingOutputPathsError, ) @@ -711,17 +712,25 @@ def test_show_from_subdir(tmp_dir, dvc, caplog): assert (subdir / "plots.html").exists() -def test_show_malformed_plot(tmp_dir, scm, dvc, caplog): +def test_show_malformed_plots(tmp_dir, scm, dvc, caplog): tmp_dir.gen("plot.json", '[{"m":1]') with pytest.raises(NoMetricsParsedError): dvc.metrics.show(targets=["plot.json"]) +def test_plots_show_no_target(tmp_dir, dvc): + with pytest.raises(MetricDoesNotExistError): + dvc.plots.show(targets=["plot.json"]) + + +def test_show_no_plots_files(tmp_dir, dvc, caplog): + with pytest.raises(NoMetricsFoundError): + dvc.metrics.show() + + @pytest.mark.parametrize("clear_before_run", [True, False]) -def test_metrics_show_overlap( - tmp_dir, dvc, run_copy_metrics, clear_before_run -): +def test_plots_show_overlap(tmp_dir, dvc, run_copy_metrics, clear_before_run): data_dir = PathInfo("data") (tmp_dir / data_dir).mkdir() diff --git a/tests/unit/test_collect.py b/tests/unit/test_collect.py index 8b709d1d9a..9237e436f5 100644 --- a/tests/unit/test_collect.py +++ b/tests/unit/test_collect.py @@ -3,7 +3,7 @@ from dvc.repo.collect import collect -def test_diff_no_file_on_target_rev(tmp_dir, scm, dvc, caplog): +def test_no_file_on_target_rev(tmp_dir, scm, dvc, caplog): with caplog.at_level(logging.WARNING, "dvc"): collect(dvc, targets=["file.yaml"], rev="current_branch")