Stone_Red/dvc
1
0
Fork 0
mirror of https://github.com/Stone-Red-Code/dvc.git synced 2026-11-09 23:10:24 +01:00
Code Issues Packages Projects Releases Wiki Activity

metrics: diff: make table more intuitive (#4655)

Browse Source 
* metrics: diff: make table more intuitive

Currently we have a "diff not supported" marker in the table, that is
used whenever dvc is not able to diff the values (e.g. no old one, no
new one or format error). It is quite ugly and it is much more intuitive
to just show both old and new values and now show that marker at all, as
if there is no old/new value it is intuitive that we can't calculate the
change. Next step is to add old/new "bad format"/etc markers for
unsupported types or malformed/missing metric files.

E.g., before:

```
Path          Metric    Value    Change
metrics.json  foo       1.2      diff not supported
```

after this PR:

```
Path          Metric    Old    New    Change
metrics.json  foo       None   1.2
```

* metrics/params: use em dash in diff table
This commit is contained in:
Ruslan Kuprieiev
2020-10-05 19:37:34 +03:00
committed by GitHub
parent d0e1c88792
commit 52fe76cefc
5 changed files with 39 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dvc/command/metrics.py
+4 -15
View File
@@ -76,7 +76,7 @@ class CmdMetricsShow(CmdMetricsBase):
return 0
def _show_diff(diff, markdown=False, no_path=False, old=False, precision=None):
def _show_diff(diff, markdown=False, no_path=False, precision=None):
from collections import OrderedDict
from dvc.utils.diff import table
@@ -96,18 +96,14 @@ def _show_diff(diff, markdown=False, no_path=False, old=False, precision=None):
for metric, change in sorted_mdiff.items():
row = [] if no_path else [fname]
row.append(metric)
if old:
row.append(_round(change.get("old")))
row.append(_round(change.get("old")))
row.append(_round(change["new"]))
row.append(_round(change.get("diff", "diff not supported")))
row.append(_round(change.get("diff")))
rows.append(row)
header = [] if no_path else ["Path"]
header.append("Metric")
if old:
header.extend(["Old", "New"])
else:
header.append("Value")
header.extend(["Old", "New"])
header.append("Change")
return table(header, rows, markdown)
@@ -133,7 +129,6 @@ class CmdMetricsDiff(CmdMetricsBase):
diff,
self.args.show_md,
self.args.no_path,
self.args.old,
precision=self.args.precision,
)
if table:
@@ -275,12 +270,6 @@ def add_parser(subparsers, parent_parser):
default=False,
help="Show tabulated output in the Markdown format (GFM).",
)
metrics_diff_parser.add_argument(
"--old",
action="store_true",
default=False,
help="Show old metric value.",
)
metrics_diff_parser.add_argument(
"--no-path",
action="store_true",
dvc/utils/diff.py
+1 -1
View File
@@ -97,7 +97,7 @@ def table(header, rows, markdown=False):
tablefmt="github" if markdown else "plain",
disable_numparse=True,
# None will be shown as "" by default, overriding
missingval="None",
missingval="—",
)
if markdown:
tests/func/metrics/test_diff.py
+1 -1
View File
@@ -185,7 +185,7 @@ def test_metrics_diff_cli(tmp_dir, scm, dvc, run_copy_metrics, caplog, capsys):
_gen(3.45678910111213)
caplog.clear()
assert main(["metrics", "diff", "HEAD~2", "--old"]) == 0
assert main(["metrics", "diff", "HEAD~2"]) == 0
(info,) = [
msg
for name, level, msg in caplog.record_tuples
tests/unit/command/test_metrics.py
+29 -49
View File
@@ -24,7 +24,6 @@ def test_metrics_diff(dvc, mocker):
"target2",
"--show-md",
"--no-path",
"--old",
"--precision",
"10",
]
@@ -51,8 +50,8 @@ def test_metrics_show_json_diff():
{"metrics.json": {"a.b.c": {"old": 1, "new": 2, "diff": 3}}}
) == textwrap.dedent(
"""\
Path Metric Value Change
metrics.json a.b.c 2 3"""
Path Metric Old New Change
metrics.json a.b.c 1 2 3"""
)
@@ -61,8 +60,8 @@ def test_metrics_show_raw_diff():
{"metrics": {"": {"old": "1", "new": "2"}}}
) == textwrap.dedent(
"""\
Path Metric Value Change
metrics 2 diff not supported"""
Path Metric Old New Change
metrics 1 2 —"""
)
@@ -71,8 +70,8 @@ def test_metrics_diff_no_diff():
{"other.json": {"a.b.d": {"old": "old", "new": "new"}}}
) == textwrap.dedent(
"""\
Path Metric Value Change
other.json a.b.d new diff not supported"""
Path Metric Old New Change
other.json a.b.d old new —"""
)
@@ -85,8 +84,8 @@ def test_metrics_diff_new_metric():
{"other.json": {"a.b.d": {"old": None, "new": "new"}}}
) == textwrap.dedent(
"""\
Path Metric Value Change
other.json a.b.d new diff not supported"""
Path Metric Old New Change
other.json a.b.d — new —"""
)
@@ -95,8 +94,8 @@ def test_metrics_diff_deleted_metric():
{"other.json": {"a.b.d": {"old": "old", "new": None}}}
) == textwrap.dedent(
"""\
Path Metric Value Change
other.json a.b.d None diff not supported"""
Path Metric Old New Change
other.json a.b.d old — —"""
)
@@ -143,14 +142,14 @@ def test_metrics_diff_precision():
assert _show_diff(diff) == textwrap.dedent(
"""\
Path Metric Value Change
other.json a.b 0.76543 0.64198"""
Path Metric Old New Change
other.json a.b 0.12346 0.76543 0.64198"""
)
assert _show_diff(diff, precision=10) == textwrap.dedent(
"""\
Path Metric Value Change
other.json a.b 0.7654321012 0.6419754012"""
Path Metric Old New Change
other.json a.b 0.1234567 0.7654321012 0.6419754012"""
)
@@ -165,18 +164,18 @@ def test_metrics_diff_sorted():
}
) == textwrap.dedent(
"""\
Path Metric Value Change
metrics.yaml a.b.c 2 1
metrics.yaml a.d.e 4 1
metrics.yaml x.b 6 1"""
Path Metric Old New Change
metrics.yaml a.b.c 1 2 1
metrics.yaml a.d.e 3 4 1
metrics.yaml x.b 5 6 1"""
)
def test_metrics_diff_markdown_empty():
assert _show_diff({}, markdown=True) == textwrap.dedent(
"""\
| Path | Metric | Value | Change |
|--------|----------|---------|----------|
| Path | Metric | Old | New | Change |
|--------|----------|-------|-------|----------|
"""
)
@@ -193,11 +192,11 @@ def test_metrics_diff_markdown():
markdown=True,
) == textwrap.dedent(
"""\
| Path | Metric | Value | Change |
|--------------|----------|---------|--------------------|
| metrics.yaml | a.b.c | 2 | 1 |
| metrics.yaml | a.d.e | 4 | 1 |
| metrics.yaml | x.b | 6 | diff not supported |
| Path | Metric | Old | New | Change |
|--------------|----------|-------|-------|----------|
| metrics.yaml | a.b.c | 1 | 2 | 1 |
| metrics.yaml | a.d.e | 3 | 4 | 1 |
| metrics.yaml | x.b | 5 | 6 | — |
"""
)
@@ -214,29 +213,10 @@ def test_metrics_diff_no_path():
no_path=True,
) == textwrap.dedent(
"""\
Metric Value Change
a.b.c 2 1
a.d.e 4 1
x.b 6 1"""
)
def test_metrics_diff_with_old():
assert _show_diff(
{
"metrics.yaml": {
"x.b": {"old": 5, "new": 6, "diff": 1},
"a.d.e": {"old": 3, "new": 4, "diff": 1},
"a.b.c": {"old": 1, "new": 2, "diff": 1},
}
},
old=True,
) == textwrap.dedent(
"""\
Path Metric Old New Change
metrics.yaml a.b.c 1 2 1
metrics.yaml a.d.e 3 4 1
metrics.yaml x.b 5 6 1"""
Metric Old New Change
a.b.c 1 2 1
a.d.e 3 4 1
x.b 5 6 1"""
)
tests/unit/command/test_params.py
+4 -4
View File
@@ -70,7 +70,7 @@ def test_params_diff_new():
) == textwrap.dedent(
"""\
Path Param Old New
params.yaml a.b.d None new"""
params.yaml a.b.d — new"""
)
@@ -80,7 +80,7 @@ def test_params_diff_deleted():
) == textwrap.dedent(
"""\
Path Param Old New
params.yaml a.b.d old None"""
params.yaml a.b.d old —"""
)
@@ -148,8 +148,8 @@ def test_params_diff_markdown():
"""\
| Path | Param | Old | New |
|-------------|---------|-------|-------|
| params.yaml | a.b.c | 1 | None |
| params.yaml | a.d.e | None | 4 |
| params.yaml | a.b.c | 1 | — |
| params.yaml | a.d.e | — | 4 |
| params.yaml | x.b | 5 | 6 |
"""
)