exp/metrics/params diff: Replaced Old/New with a_rev/b_rev (#6814)

* Replaced Old/New with a_rev/b_rev

* Fixed tests
This commit is contained in:
David de la Iglesia Castro
2021-10-18 21:09:02 +03:00
committed by GitHub
parent 7bef6fd64f
commit 4f16852005
8 changed files with 66 additions and 34 deletions
+3 -1
View File
@@ -579,6 +579,8 @@ class CmdExperimentsDiff(CmdBase):
old=self.args.old,
on_empty_diff="diff not supported",
precision=precision if key == "metrics" else None,
a_rev=self.args.a_rev,
b_rev=self.args.b_rev,
)
return 0
@@ -1070,7 +1072,7 @@ def add_parser(subparsers, parent_parser):
"--old",
action="store_true",
default=False,
help="Show old metric/param value.",
help="Show old ('a_rev') metric/param value.",
)
experiments_diff_parser.add_argument(
"--no-path",
+2
View File
@@ -75,6 +75,8 @@ class CmdMetricsDiff(CmdMetricsBase):
no_path=self.args.no_path,
precision=self.args.precision or DEFAULT_PRECISION,
round_digits=True,
a_rev=self.args.a_rev,
b_rev=self.args.b_rev,
)
return 0
+2
View File
@@ -36,6 +36,8 @@ class CmdParamsDiff(CmdBase):
markdown=self.args.markdown,
no_path=self.args.no_path,
show_changes=False,
a_rev=self.args.a_rev,
b_rev=self.args.b_rev,
)
return 0
+11 -3
View File
@@ -211,8 +211,12 @@ def diff_table(
precision: int = None,
round_digits: bool = False,
on_empty_diff: str = None,
a_rev: str = None,
b_rev: str = None,
) -> TabularData:
headers: List[str] = ["Path", title, "Old", "New", "Change"]
a_rev = a_rev or "HEAD"
b_rev = b_rev or "workspace"
headers: List[str] = ["Path", title, a_rev, b_rev, "Change"]
fill_value = "-"
td = TabularData(headers, fill_value=fill_value)
@@ -240,8 +244,8 @@ def diff_table(
td.drop("Change")
if not old:
td.drop("Old")
td.rename("New", "Value")
td.drop(a_rev)
td.rename(b_rev, "Value")
return td
@@ -256,6 +260,8 @@ def show_diff(
round_digits: bool = False,
on_empty_diff: str = None,
markdown: bool = False,
a_rev: str = None,
b_rev: str = None,
) -> None:
td = diff_table(
diff,
@@ -266,6 +272,8 @@ def show_diff(
precision=precision,
round_digits=round_digits,
on_empty_diff=on_empty_diff,
a_rev=a_rev,
b_rev=b_rev,
)
td.render(markdown=markdown)
+3 -2
View File
@@ -187,9 +187,10 @@ def test_metrics_diff_cli(tmp_dir, scm, dvc, run_copy_metrics, caplog, capsys):
assert main(["metrics", "diff", "HEAD~2"]) == 0
captured = capsys.readouterr()
assert captured.out == (
"Path Metric Old New Change\n"
"m.yaml foo 1.23457 3.45679 2.22222\n"
"Path Metric HEAD~2 workspace Change\n"
"m.yaml foo 1.23457 3.45679 2.22222\n"
)
+2
View File
@@ -47,6 +47,8 @@ def test_metrics_diff(dvc, mocker, capsys):
precision=5,
markdown=True,
round_digits=True,
a_rev="HEAD~10",
b_rev="HEAD~1",
)
+7 -1
View File
@@ -56,7 +56,13 @@ def test_params_diff_from_cli(dvc, mocker):
deps=False,
)
show_diff_mock.assert_called_once_with(
{}, title="Param", markdown=False, no_path=False, show_changes=False
{},
title="Param",
markdown=False,
no_path=False,
show_changes=False,
a_rev=None,
b_rev=None,
)
+36 -27
View File
@@ -16,8 +16,8 @@ def test_diff_table(title):
{
"Path": "metrics.json",
title: "a.b.c",
"Old": "1",
"New": "2",
"HEAD": "1",
"workspace": "2",
"Change": "3",
}
]
@@ -46,7 +46,7 @@ def test_no_path():
no_path=True,
)
assert td.as_dict() == [
{"Metric": "a.b.c", "Old": "1", "New": "2", "Change": "3"}
{"Metric": "a.b.c", "HEAD": "1", "workspace": "2", "Change": "3"}
]
@@ -57,7 +57,12 @@ def test_do_not_show_changes():
show_changes=False,
)
assert td.as_dict() == [
{"Path": "metrics.json", "Metric": "a.b.c", "Old": "1", "New": "2"}
{
"Path": "metrics.json",
"Metric": "a.b.c",
"HEAD": "1",
"workspace": "2",
}
]
@@ -72,8 +77,8 @@ def test_diff_table_precision():
{
"Path": "metrics.json",
"Metric": "a.b.c",
"Old": "1.12",
"New": "2.23",
"HEAD": "1.12",
"workspace": "2.23",
"Change": "3.35",
}
]
@@ -90,8 +95,8 @@ def test_diff_table_rounding():
{
"Path": "metrics.json",
"Metric": "a.b.c",
"Old": "1.123",
"New": "2.235",
"HEAD": "1.123",
"workspace": "2.235",
"Change": "3.346",
}
]
@@ -110,8 +115,8 @@ def test_diff_unsupported_diff_message(extra, expected):
{
"Path": "metrics.json",
"Metric": "",
"Old": "1",
"New": "2",
"HEAD": "1",
"workspace": "2",
"Change": expected,
}
]
@@ -125,8 +130,8 @@ def test_diff_new():
{
"Path": "param.json",
"Param": "a.b.d",
"Old": "-",
"New": "new",
"HEAD": "-",
"workspace": "new",
"Change": "-",
}
]
@@ -140,8 +145,8 @@ def test_diff_old_deleted():
{
"Path": "metric.json",
"Metric": "a.b.d",
"Old": "old",
"New": "-",
"HEAD": "old",
"workspace": "-",
"Change": "-",
}
]
@@ -172,8 +177,8 @@ def test_diff_falsey_values():
{
"Path": "metrics.yaml",
"Metric": "x.b",
"Old": "0",
"New": "0.0",
"HEAD": "0",
"workspace": "0.0",
"Change": "0.0",
}
]
@@ -191,8 +196,8 @@ def test_diff_list(composite, expected):
{
"Path": "params.yaml",
"Param": "a.b.c",
"Old": "1",
"New": expected,
"HEAD": "1",
"workspace": expected,
"Change": "-",
}
]
@@ -214,6 +219,8 @@ def test_diff_mocked(mocker, markdown):
on_empty_diff=None,
show_changes=True,
round_digits=False,
a_rev=None,
b_rev=None,
)
ret.render.assert_called_once_with(markdown=markdown)
@@ -230,12 +237,13 @@ def test_diff_default(capsys):
"Metric",
)
out, _ = capsys.readouterr()
assert out == 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 -
Path Metric HEAD workspace Change
metrics.yaml a.b.c 1 2 1
metrics.yaml a.d.e 3 4 1
metrics.yaml x.b 5 6 -
"""
)
@@ -253,13 +261,14 @@ def test_metrics_diff_md(capsys):
markdown=True,
)
out, _ = capsys.readouterr()
assert out == 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 | - |
| Path | Metric | HEAD | workspace | Change |
|--------------|----------|--------|-------------|----------|
| metrics.yaml | a.b.c | 1 | 2 | 1 |
| metrics.yaml | a.d.e | 3 | 4 | 1 |
| metrics.yaml | x.b | 5 | 6 | - |
"""
)