Merge pull request #3051 from efiop/2995

metrics: introduce `diff`
This commit is contained in:
Ruslan Kuprieiev
2020-01-14 14:18:40 +02:00
committed by GitHub
8 changed files with 390 additions and 19 deletions
+121 -1
View File
@@ -26,7 +26,12 @@ def show_metrics(metrics, all_branches=False, all_tags=False):
logger.info("{branch}:".format(branch=branch))
for fname, metric in val.items():
lines = metric if type(metric) is list else metric.splitlines()
if isinstance(metric, dict):
lines = list(metric.values())
elif isinstance(metric, list):
lines = metric
else:
lines = metric.splitlines()
if len(lines) > 1:
logger.info("\t{fname}:".format(fname=fname))
@@ -100,6 +105,59 @@ class CmdMetricsRemove(CmdBase):
return 0
def _show_diff(diff):
from texttable import Texttable
if not diff:
return "No changes."
table = Texttable()
# remove borders to make it easier for users to copy stuff
table.set_chars(("", "", "", ""))
table.set_deco(0)
rows = [["Path", "Metric", "Value", "Change"]]
for fname, mdiff in diff.items():
for metric, change in mdiff.items():
rows.append(
[
fname,
metric,
change["new"],
change.get("diff", "diff not supported"),
]
)
table.add_rows(rows)
return table.draw()
class CmdMetricsDiff(CmdBase):
def run(self):
try:
diff = self.repo.metrics.diff(
a_ref=self.args.a_ref,
b_ref=self.args.b_ref,
targets=self.args.targets,
typ=self.args.type,
xpath=self.args.xpath,
recursive=self.args.recursive,
)
if self.args.show_json:
import json
logger.info(json.dumps(diff))
else:
logger.info(_show_diff(diff))
except DvcException:
logger.exception("failed to show metrics diff")
return 1
return 0
def add_parser(subparsers, parent_parser):
METRICS_HELP = "Commands to add, manage, collect and display metrics."
@@ -214,3 +272,65 @@ def add_parser(subparsers, parent_parser):
)
metrics_remove_parser.add_argument("path", help="Path to a metric file.")
metrics_remove_parser.set_defaults(func=CmdMetricsRemove)
METRICS_DIFF_HELP = "Output metric values."
metrics_diff_parser = metrics_subparsers.add_parser(
"diff",
parents=[parent_parser],
description=append_doc_link(METRICS_DIFF_HELP, "metrics/diff"),
help=METRICS_DIFF_HELP,
formatter_class=argparse.RawDescriptionHelpFormatter,
)
metrics_diff_parser.add_argument(
"a_ref",
nargs="?",
help=(
"Git reference from which diff is calculated. "
"If omitted `HEAD`(latest commit) is used."
),
)
metrics_diff_parser.add_argument(
"b_ref",
nargs="?",
help=(
"Git reference to which diff is calculated. "
"If omitted current working tree is used."
),
)
metrics_diff_parser.add_argument(
"--targets",
nargs="*",
help=(
"Metric files or directories (see -R) to show diff for. "
"Shows diff for all metric files by default."
),
)
metrics_diff_parser.add_argument(
"-t",
"--type",
help=(
"Type of metrics (json/tsv/htsv/csv/hcsv). "
"It can be detected by the file extension automatically. "
"Unsupported types will be treated as raw."
),
)
metrics_diff_parser.add_argument(
"-x", "--xpath", help="json/tsv/htsv/csv/hcsv path."
)
metrics_diff_parser.add_argument(
"-R",
"--recursive",
action="store_true",
default=False,
help=(
"If any target is a directory, recursively search and process "
"metric files."
),
)
metrics_diff_parser.add_argument(
"--show-json",
action="store_true",
default=False,
help="Show output in JSON format.",
)
metrics_diff_parser.set_defaults(func=CmdMetricsDiff)
+3 -3
View File
@@ -4,7 +4,7 @@ from dvc.scm.tree import WorkingTree
def brancher( # noqa: E302
self, all_branches=False, all_tags=False, all_commits=False
self, revs=None, all_branches=False, all_tags=False, all_commits=False
):
"""Generator that iterates over specified revisions.
@@ -20,12 +20,12 @@ def brancher( # noqa: E302
- empty string it there is no branches to iterate over
- "Working Tree" if there are uncommitted changes in the SCM repo
"""
if not any([all_branches, all_tags, all_commits]):
if not any([revs, all_branches, all_tags, all_commits]):
yield ""
return
saved_tree = self.tree
revs = []
revs = revs or []
scm = self.scm
+5
View File
@@ -21,3 +21,8 @@ class Metrics(object):
from dvc.repo.metrics.remove import remove
return remove(self.repo, *args, **kwargs)
def diff(self, *args, **kwargs):
from .diff import diff
return diff(self.repo, *args, **kwargs)
+105
View File
@@ -0,0 +1,105 @@
import json
from collections import defaultdict
from flatten_dict import flatten
from dvc.exceptions import NoMetricsError
def _parse(raw):
if isinstance(raw, (dict, list, int, float)):
return raw
assert isinstance(raw, str)
try:
return json.loads(raw)
except json.JSONDecodeError:
return raw
def _diff_vals(old, new):
if (
isinstance(new, list)
and isinstance(old, list)
and len(old) == len(new) == 1
):
return _diff_vals(old[0], new[0])
if old == new:
return {}
res = {"old": old, "new": new}
if isinstance(new, (int, float)) and isinstance(old, (int, float)):
res["diff"] = new - old
return res
# dot_reducer is not released yet (flatten-dict > 0.2.0)
def _dot(k1, k2):
if k1 is None:
return k2
return "{0}.{1}".format(k1, k2)
def _diff_dicts(old_dict, new_dict):
old_default = None
new_default = None
if isinstance(new_dict, dict):
new = flatten(new_dict, reducer=_dot)
else:
new = defaultdict(lambda: "not a dict")
new_default = "unable to parse"
if isinstance(old_dict, dict):
old = flatten(old_dict, reducer=_dot)
else:
old = defaultdict(lambda: "not a dict")
old_default = "unable to parse"
res = defaultdict(dict)
xpaths = set(old.keys())
xpaths.update(set(new.keys()))
for xpath in xpaths:
old_val = old.get(xpath, old_default)
new_val = new.get(xpath, new_default)
val_diff = _diff_vals(old_val, new_val)
if val_diff:
res[xpath] = val_diff
return dict(res)
def _diff(old_raw, new_raw):
old = _parse(old_raw)
new = _parse(new_raw)
if isinstance(new, dict) or isinstance(old, dict):
return _diff_dicts(old, new)
return {"": _diff_vals(old, new)}
def _get_metrics(repo, *args, rev=None, **kwargs):
try:
metrics = repo.metrics.show(
*args, **kwargs, revs=[rev] if rev else None
)
return metrics[rev or ""]
except NoMetricsError:
return {}
def diff(repo, *args, a_ref=None, b_ref=None, **kwargs):
old = _get_metrics(repo, *args, **kwargs, rev=(a_ref or "HEAD"))
new = _get_metrics(repo, *args, **kwargs, rev=b_ref)
paths = set(old.keys())
paths.update(set(new.keys()))
res = defaultdict(dict)
for path in paths:
path_diff = _diff(old[path], new[path])
if path_diff:
res[path] = path_diff
return dict(res)
+5 -2
View File
@@ -20,7 +20,7 @@ logger = logging.getLogger(__name__)
def _read_metric_json(fd, json_path):
parser = parse(json_path)
return [x.value for x in parser.find(json.load(fd))]
return {str(x.full_path): x.value for x in parser.find(json.load(fd))}
def _get_values(row):
@@ -266,6 +266,7 @@ def show(
all_branches=False,
all_tags=False,
recursive=False,
revs=None,
):
res = {}
found = set()
@@ -274,7 +275,9 @@ def show(
# Iterate once to call `_collect_metrics` on all the stages
targets = [None]
for branch in repo.brancher(all_branches=all_branches, all_tags=all_tags):
for branch in repo.brancher(
revs=revs, all_branches=all_branches, all_tags=all_tags
):
metrics = {}
for target in targets:
+2
View File
@@ -75,6 +75,8 @@ install_requires = [
"win-unicode-console>=0.5; sys_platform == 'win32'",
"pywin32>=225; sys_platform == 'win32'",
"networkx>=2.1,<2.4",
"flatten-dict>=0.2.0",
"texttable>=0.5.2",
]
+82 -13
View File
@@ -4,6 +4,8 @@ import json
import logging
import os
import pytest
from dvc.exceptions import DvcException
from dvc.exceptions import NoMetricsError
from dvc.main import main
@@ -113,9 +115,9 @@ class TestMetrics(TestMetricsBase):
["metric_json"], typ="json", xpath="branch", all_branches=True
)
self.assertEqual(len(ret), 3)
self.assertSequenceEqual(ret["foo"]["metric_json"], ["foo"])
self.assertSequenceEqual(ret["bar"]["metric_json"], ["bar"])
self.assertSequenceEqual(ret["baz"]["metric_json"], ["baz"])
self.assertSequenceEqual(ret["foo"]["metric_json"], {"branch": "foo"})
self.assertSequenceEqual(ret["bar"]["metric_json"], {"branch": "bar"})
self.assertSequenceEqual(ret["baz"]["metric_json"], {"branch": "baz"})
ret = self.dvc.metrics.show(
["metric_tsv"], typ="tsv", xpath="0,0", all_branches=True
@@ -158,13 +160,13 @@ class TestMetrics(TestMetricsBase):
self.assertEqual(len(ret), 1)
self.assertSequenceEqual(
ret["foo"]["metric_json_ext"],
[
{
{
"metrics.[0]": {
"dataset": "train",
"deviation_mse": 0.173461,
"value_mse": 0.421601,
}
],
},
)
self.assertRaises(KeyError, lambda: ret["bar"])
self.assertRaises(KeyError, lambda: ret["baz"])
@@ -723,9 +725,9 @@ class TestCachedMetrics(TestDvcGit):
self.assertEqual(
res,
{
"master": {"metrics.json": ["master"]},
"one": {"metrics.json": ["one"]},
"two": {"metrics.json": ["two"]},
"master": {"metrics.json": {"metrics": "master"}},
"one": {"metrics.json": {"metrics": "one"}},
"two": {"metrics.json": {"metrics": "two"}},
},
)
@@ -736,9 +738,9 @@ class TestCachedMetrics(TestDvcGit):
self.assertEqual(
res,
{
"master": {"metrics.json": ["master"]},
"one": {"metrics.json": ["one"]},
"two": {"metrics.json": ["two"]},
"master": {"metrics.json": {"metrics": "master"}},
"one": {"metrics.json": {"metrics": "one"}},
"two": {"metrics.json": {"metrics": "two"}},
},
)
@@ -802,6 +804,10 @@ class TestMetricsType(TestDvcGit):
for branch in self.branches:
if isinstance(ret[branch][file_name], list):
self.assertSequenceEqual(ret[branch][file_name], [branch])
elif isinstance(ret[branch][file_name], dict):
self.assertSequenceEqual(
ret[branch][file_name], {"branch": branch}
)
else:
self.assertSequenceEqual(ret[branch][file_name], branch)
@@ -834,7 +840,7 @@ def test_show_xpath_should_override_stage_xpath(tmp_dir, dvc):
dvc.run(cmd="", overwrite=True, metrics=["metric"])
dvc.metrics.modify("metric", typ="json", xpath="m2")
assert dvc.metrics.show(xpath="m1") == {"": {"metric": [0.1]}}
assert dvc.metrics.show(xpath="m1") == {"": {"metric": {"m1": 0.1}}}
def test_show_multiple_outputs(tmp_dir, dvc, caplog):
@@ -871,3 +877,66 @@ def test_show_multiple_outputs(tmp_dir, dvc, caplog):
"the following metrics do not exist, "
"are not metric files or are malformed: 'not-found'"
) in caplog.text
def test_metrics_diff_raw(tmp_dir, scm, dvc):
def _gen(val):
tmp_dir.gen({"metrics": val})
dvc.run(cmd="", metrics=["metrics"])
dvc.scm.add(["metrics.dvc"])
dvc.scm.commit(str(val))
_gen("raw 1")
_gen("raw 2")
_gen("raw 3")
assert dvc.metrics.diff(a_ref="HEAD~2") == {
"metrics": {"": {"old": "raw 1", "new": "raw 3"}}
}
@pytest.mark.parametrize("xpath", [True, False])
def test_metrics_diff_json(tmp_dir, scm, dvc, xpath):
def _gen(val):
metrics = {"a": {"b": {"c": val, "d": 1, "e": str(val)}}}
tmp_dir.gen({"m.json": json.dumps(metrics)})
dvc.run(cmd="", metrics=["m.json"])
dvc.metrics.modify("m.json", typ="json")
if xpath:
dvc.metrics.modify("m.json", xpath="a.b.c")
dvc.scm.add(["m.json.dvc"])
dvc.scm.commit(str(val))
_gen(1)
_gen(2)
_gen(3)
expected = {"m.json": {"a.b.c": {"old": 1, "new": 3, "diff": 2}}}
if not xpath:
expected["m.json"]["a.b.e"] = {"old": "1", "new": "3"}
assert expected == dvc.metrics.diff(a_ref="HEAD~2")
def test_metrics_diff_broken_json(tmp_dir, scm, dvc):
metrics = {"a": {"b": {"c": 1, "d": 1, "e": "3"}}}
tmp_dir.gen({"m.json": json.dumps(metrics)})
dvc.run(cmd="", metrics_no_cache=["m.json"])
dvc.scm.add(["m.json.dvc", "m.json"])
dvc.scm.commit("add metrics")
(tmp_dir / "m.json").write_text(json.dumps(metrics) + "ma\nlformed\n")
assert dvc.metrics.diff() == {
"m.json": {
"a.b.c": {"old": 1, "new": "unable to parse"},
"a.b.d": {"old": 1, "new": "unable to parse"},
"a.b.e": {"old": "3", "new": "unable to parse"},
}
}
def test_metrics_diff_no_metrics(tmp_dir, scm, dvc):
tmp_dir.scm_gen({"foo": "foo"}, commit="add foo")
assert dvc.metrics.diff(a_ref="HEAD~1") == {}
+67
View File
@@ -0,0 +1,67 @@
from dvc.cli import parse_args
from dvc.command.metrics import CmdMetricsDiff, _show_diff
def test_metrics_diff(dvc, mocker):
cli_args = parse_args(
[
"metrics",
"diff",
"HEAD~10",
"HEAD~1",
"-t",
"json",
"-x",
"x.path",
"-R",
"--show-json",
"--targets",
"target1",
"target2",
]
)
assert cli_args.func == CmdMetricsDiff
cmd = cli_args.func(cli_args)
m = mocker.patch("dvc.repo.metrics.diff.diff", return_value={})
assert cmd.run() == 0
m.assert_called_once_with(
cmd.repo,
targets=["target1", "target2"],
a_ref="HEAD~10",
b_ref="HEAD~1",
typ="json",
xpath="x.path",
recursive=True,
)
def test_metrics_show_json_diff():
assert _show_diff(
{"metrics.json": {"a.b.c": {"old": 1, "new": 2, "diff": 3}}}
) == (
" Path Metric Value Change\n"
"metrics.json a.b.c 2 3 "
)
def test_metrics_show_raw_diff():
assert _show_diff({"metrics": {"": {"old": "1", "new": "2"}}}) == (
" Path Metric Value Change \n"
"metrics 2 diff not supported"
)
def test_metrics_diff_no_diff():
assert _show_diff(
{"other.json": {"a.b.d": {"old": "old", "new": "new"}}}
) == (
" Path Metric Value Change \n"
"other.json a.b.d new diff not supported"
)
def test_metrics_diff_no_changes():
assert _show_diff({}) == "No changes."