Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

params: introduce --targets #5122

Merged
merged 8 commits into from
Dec 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions dvc/command/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -30,11 +31,14 @@ def _show_diff(diff, markdown=False, no_path=False):


class CmdParamsDiff(CmdBase):
UNINITIALIZED = True

def run(self):
try:
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,
)

Expand Down Expand Up @@ -91,6 +95,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="<path>",
).complete = completion.FILE
params_diff_parser.add_argument(
"--all",
action="store_true",
Expand Down
17 changes: 14 additions & 3 deletions dvc/exceptions.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -170,19 +172,19 @@ 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 "
"details."
)


class NoMetricsFoundError(NoMetricsError):
class NoMetricsFoundError(MetricsError):
def __init__(self, command, run_options):
super().__init__(
f"No {command} files in this repository. "
Expand All @@ -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__(
Expand Down
4 changes: 2 additions & 2 deletions dvc/repo/metrics/diff.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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 {}


Expand Down
8 changes: 7 additions & 1 deletion dvc/repo/metrics/show.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")

Expand Down
30 changes: 21 additions & 9 deletions dvc/repo/params/show.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from dvc.path_info import PathInfo
from dvc.repo import locked
from dvc.repo.collect import collect
from dvc.scm.base import SCMError
from dvc.stage import PipelineStage
from dvc.utils.serialize import LOADERS, ParseError

Expand All @@ -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):
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions dvc/repo/plots/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from dvc.exceptions import (
DvcException,
MetricDoesNotExistError,
NoMetricsFoundError,
NoMetricsParsedError,
)
Expand Down Expand Up @@ -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)

Expand All @@ -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:
Expand Down
5 changes: 0 additions & 5 deletions dvc/repo/plots/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Empty file.
108 changes: 0 additions & 108 deletions tests/func/metrics/test_common.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,16 @@

import pytest

from dvc.exceptions import (
MetricDoesNotExistError,
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):
Expand Down Expand Up @@ -166,3 +174,55 @@ 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_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()


@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()
Loading