From 4fc086c9fc1cd1747742835180d5a2d454605bb2 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Thu, 7 May 2020 20:02:47 +0545 Subject: [PATCH 1/4] params/metrics: diff: implement tabulated markdown output Via `--show-md` on `diff`s. --- dvc/command/metrics.py | 12 +++++++++--- dvc/command/params.py | 21 +++++++++++++++++---- dvc/utils/diff.py | 24 +++++++++--------------- setup.py | 2 +- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index 19440121fb..15cc756d65 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -87,7 +87,7 @@ def run(self): return 0 -def _show_diff(diff): +def _show_diff(diff, markdown): from collections import OrderedDict from dvc.utils.diff import table @@ -105,7 +105,7 @@ def _show_diff(diff): ] ) - return table(["Path", "Metric", "Value", "Change"], rows) + return table(["Path", "Metric", "Value", "Change"], rows, markdown) class CmdMetricsDiff(CmdBase): @@ -124,7 +124,7 @@ def run(self): logger.info(json.dumps(diff)) else: - table = _show_diff(diff) + table = _show_diff(diff, self.args.show_md) if table: logger.info(table) @@ -263,6 +263,12 @@ def add_parser(subparsers, parent_parser): default=False, help="Show output in JSON format.", ) + metrics_diff_parser.add_argument( + "--show-md", + action="store_true", + default=False, + help="Show tabulated output in the Markdown format (GFM).", + ) metrics_diff_parser.set_defaults(func=CmdMetricsDiff) METRICS_REMOVE_HELP = "Remove metric mark on a DVC-tracked file." diff --git a/dvc/command/params.py b/dvc/command/params.py index 881b2cf58d..3eacccb940 100644 --- a/dvc/command/params.py +++ b/dvc/command/params.py @@ -8,16 +8,23 @@ logger = logging.getLogger(__name__) -def _show_diff(diff): +def _show_diff(diff, markdown): from dvc.utils.diff import table rows = [] for fname, pdiff in diff.items(): sorted_pdiff = OrderedDict(sorted(pdiff.items())) for param, change in sorted_pdiff.items(): - rows.append([fname, param, change["old"], change["new"]]) + rows.append( + [ + fname, + param, + change["old"] or "None", + change["new"] or "None", + ] + ) - return table(["Path", "Param", "Old", "New"], rows) + return table(["Path", "Param", "Old", "New"], rows, markdown) class CmdParamsDiff(CmdBase): @@ -34,7 +41,7 @@ def run(self): logger.info(json.dumps(diff)) else: - table = _show_diff(diff) + table = _show_diff(diff, self.args.show_md) if table: logger.info(table) @@ -94,4 +101,10 @@ def add_parser(subparsers, parent_parser): default=False, help="Show output in JSON format.", ) + params_diff_parser.add_argument( + "--show-md", + action="store_true", + default=False, + help="Show tabulated output in the Markdown format (GFM).", + ) params_diff_parser.set_defaults(func=CmdParamsDiff) diff --git a/dvc/utils/diff.py b/dvc/utils/diff.py index 9b57f623bd..9ae68ec1c5 100644 --- a/dvc/utils/diff.py +++ b/dvc/utils/diff.py @@ -85,24 +85,18 @@ def diff(old, new, with_unchanged=False): return dict(res) -def table(header, rows): - from texttable import Texttable +def table(header, rows, markdown=False): + from tabulate import tabulate - if not rows: + if not rows and not markdown: return "" - t = Texttable() - - # disable automatic formatting - t.set_cols_dtype(["t"] * len(header)) - - # remove borders to make it easier for users to copy stuff - t.set_chars([""] * len(header)) - t.set_deco(0) - - t.add_rows([header] + rows) - - return t.draw() + return tabulate( + rows, + header, + tablefmt="github" if markdown else "plain", + disable_numparse=True, + ) def format_dict(d): diff --git a/setup.py b/setup.py index 567715eb41..08f6ec959b 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,7 @@ def run(self): "pydot>=1.2.4", "speedcopy>=2.0.1; python_version < '3.8' and sys_platform == 'win32'", "flatten_json>=0.1.6", - "texttable>=0.5.2", + "tabulate>=0.8.7", "pygtrie==2.3.2", "dpath>=2.0.1,<3", ] From 194e07d44a29a5d6c0fe72277dac14ecf71952b4 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Fri, 8 May 2020 15:14:46 +0545 Subject: [PATCH 2/4] table: show None on missing val --- dvc/command/metrics.py | 2 +- dvc/command/params.py | 11 ++--------- dvc/utils/diff.py | 2 ++ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index 15cc756d65..6a4e50ad09 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -87,7 +87,7 @@ def run(self): return 0 -def _show_diff(diff, markdown): +def _show_diff(diff, markdown=False): from collections import OrderedDict from dvc.utils.diff import table diff --git a/dvc/command/params.py b/dvc/command/params.py index 3eacccb940..ddf954b324 100644 --- a/dvc/command/params.py +++ b/dvc/command/params.py @@ -8,21 +8,14 @@ logger = logging.getLogger(__name__) -def _show_diff(diff, markdown): +def _show_diff(diff, markdown=False): from dvc.utils.diff import table rows = [] for fname, pdiff in diff.items(): sorted_pdiff = OrderedDict(sorted(pdiff.items())) for param, change in sorted_pdiff.items(): - rows.append( - [ - fname, - param, - change["old"] or "None", - change["new"] or "None", - ] - ) + rows.append([fname, param, change["old"], change["new"]]) return table(["Path", "Param", "Old", "New"], rows, markdown) diff --git a/dvc/utils/diff.py b/dvc/utils/diff.py index 9ae68ec1c5..b412aafeba 100644 --- a/dvc/utils/diff.py +++ b/dvc/utils/diff.py @@ -96,6 +96,8 @@ def table(header, rows, markdown=False): header, tablefmt="github" if markdown else "plain", disable_numparse=True, + # None will be shown as "" by default, overriding + missingval="None", ) From 96adc53a252d8ac2ac17d0d8911bb10dafc3a709 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Fri, 8 May 2020 15:15:36 +0545 Subject: [PATCH 3/4] tests: use `dedent`, add tests for markdown --- tests/unit/command/test_metrics.py | 85 ++++++++++++++++++++++-------- tests/unit/command/test_params.py | 80 +++++++++++++++++++++------- 2 files changed, 123 insertions(+), 42 deletions(-) diff --git a/tests/unit/command/test_metrics.py b/tests/unit/command/test_metrics.py index 9b1c5a266b..cd4bb7c2ae 100644 --- a/tests/unit/command/test_metrics.py +++ b/tests/unit/command/test_metrics.py @@ -1,3 +1,5 @@ +import textwrap + from dvc.cli import parse_args from dvc.command.metrics import CmdMetricsDiff, CmdMetricsShow, _show_diff @@ -37,25 +39,30 @@ def test_metrics_diff(dvc, mocker): 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 " + ) == textwrap.dedent( + """\ + Path Metric Value Change + 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" + assert _show_diff( + {"metrics": {"": {"old": "1", "new": "2"}}} + ) == textwrap.dedent( + """\ + Path Metric Value Change + 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" + ) == textwrap.dedent( + """\ + Path Metric Value Change + other.json a.b.d new diff not supported""" ) @@ -66,18 +73,20 @@ def test_metrics_diff_no_changes(): def test_metrics_diff_new_metric(): assert _show_diff( {"other.json": {"a.b.d": {"old": None, "new": "new"}}} - ) == ( - " Path Metric Value Change \n" - "other.json a.b.d new diff not supported" + ) == textwrap.dedent( + """\ + Path Metric Value Change + other.json a.b.d new diff not supported""" ) def test_metrics_diff_deleted_metric(): assert _show_diff( {"other.json": {"a.b.d": {"old": "old", "new": None}}} - ) == ( - " Path Metric Value Change \n" - "other.json a.b.d None diff not supported" + ) == textwrap.dedent( + """\ + Path Metric Value Change + other.json a.b.d None diff not supported""" ) @@ -114,9 +123,10 @@ def test_metrics_show(dvc, mocker): def test_metrics_diff_prec(): assert _show_diff( {"other.json": {"a.b": {"old": 0.0042, "new": 0.0043, "diff": 0.0001}}} - ) == ( - " Path Metric Value Change\n" - "other.json a.b 0.0043 0.0001" + ) == textwrap.dedent( + """\ + Path Metric Value Change + other.json a.b 0.0043 0.0001""" ) @@ -129,9 +139,38 @@ def test_metrics_diff_sorted(): "a.b.c": {"old": 1, "new": 2, "diff": 1}, } } - ) == ( - " Path Metric Value Change\n" - "metrics.yaml a.b.c 2 1 \n" - "metrics.yaml a.d.e 4 1 \n" - "metrics.yaml x.b 6 1 " + ) == 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""" + ) + + +def test_metrics_diff_markdown_empty(): + assert _show_diff({}, markdown=True) == textwrap.dedent( + """\ + | Path | Metric | Value | Change | + |--------|----------|---------|----------|""" + ) + + +def test_metrics_diff_markdown(): + assert _show_diff( + { + "metrics.yaml": { + "x.b": {"old": 5, "new": 6}, + "a.d.e": {"old": 3, "new": 4, "diff": 1}, + "a.b.c": {"old": 1, "new": 2, "diff": 1}, + } + }, + 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 |""" ) diff --git a/tests/unit/command/test_params.py b/tests/unit/command/test_params.py index bb0b6f1368..8ebfac5fdc 100644 --- a/tests/unit/command/test_params.py +++ b/tests/unit/command/test_params.py @@ -1,4 +1,5 @@ import logging +import textwrap from dvc.cli import parse_args from dvc.command.params import CmdParamsDiff, _show_diff @@ -21,25 +22,32 @@ def test_params_diff(dvc, mocker): def test_params_diff_changed(): - assert _show_diff({"params.yaml": {"a.b.c": {"old": 1, "new": 2}}}) == ( - " Path Param Old New\n" "params.yaml a.b.c 1 2 " + assert _show_diff( + {"params.yaml": {"a.b.c": {"old": 1, "new": 2}}} + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml a.b.c 1 2""" ) def test_params_diff_list(): assert _show_diff( {"params.yaml": {"a.b.c": {"old": 1, "new": [2, 3]}}} - ) == ( - " Path Param Old New \n" - "params.yaml a.b.c 1 [2, 3]" + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml a.b.c 1 [2, 3]""" ) def test_params_diff_unchanged(): assert _show_diff( {"params.yaml": {"a.b.d": {"old": "old", "new": "new"}}} - ) == ( - " Path Param Old New\n" "params.yaml a.b.d old new" + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml a.b.d old new""" ) @@ -50,25 +58,30 @@ def test_params_diff_no_changes(): def test_params_diff_new(): assert _show_diff( {"params.yaml": {"a.b.d": {"old": None, "new": "new"}}} - ) == ( - " Path Param Old New\n" "params.yaml a.b.d None new" + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml a.b.d None new""" ) def test_params_diff_deleted(): assert _show_diff( {"params.yaml": {"a.b.d": {"old": "old", "new": None}}} - ) == ( - " Path Param Old New \n" "params.yaml a.b.d old None" + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml a.b.d old None""" ) def test_params_diff_prec(): assert _show_diff( {"params.yaml": {"train.lr": {"old": 0.0042, "new": 0.0043}}} - ) == ( - " Path Param Old New \n" - "params.yaml train.lr 0.0042 0.0043" + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml train.lr 0.0042 0.0043""" ) @@ -94,9 +107,38 @@ def test_params_diff_sorted(): "a.b.c": {"old": 1, "new": 2}, } } - ) == ( - " Path Param Old New\n" - "params.yaml a.b.c 1 2 \n" - "params.yaml a.d.e 3 4 \n" - "params.yaml x.b 5 6 " + ) == textwrap.dedent( + """\ + Path Param Old New + params.yaml a.b.c 1 2 + params.yaml a.d.e 3 4 + params.yaml x.b 5 6""" + ) + + +def test_params_diff_markdown_empty(): + assert _show_diff({}, markdown=True) == textwrap.dedent( + """\ + | Path | Param | Old | New | + |--------|---------|-------|-------|""" + ) + + +def test_params_diff_markdown(): + assert _show_diff( + { + "params.yaml": { + "x.b": {"old": 5, "new": 6}, + "a.d.e": {"old": None, "new": 4}, + "a.b.c": {"old": 1, "new": None}, + } + }, + markdown=True, + ) == textwrap.dedent( + """\ + | Path | Param | Old | New | + |-------------|---------|-------|-------| + | params.yaml | a.b.c | 1 | None | + | params.yaml | a.d.e | None | 4 | + | params.yaml | x.b | 5 | 6 |""" ) From f9c0a2d33b172ce6162e0c76d5b8bb3623471746 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 9 May 2020 02:34:44 +0300 Subject: [PATCH 4/4] diff: add --show-md --- dvc/command/diff.py | 23 +++++++++++++++++++++++ tests/unit/command/test_diff.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 0c42f9c287..45594f4458 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -11,6 +11,21 @@ logger = logging.getLogger(__name__) +def _show_md(diff): + from dvc.utils.diff import table + + rows = [] + for status in ["added", "deleted", "modified"]: + entries = diff.get(status, []) + if not entries: + continue + paths = sorted([entry["path"] for entry in entries]) + for path in paths: + rows.append([status, path]) + + return table(["Status", "Path"], rows, True) + + class CmdDiff(CmdBase): @staticmethod def _format(diff): @@ -103,6 +118,8 @@ def run(self): if self.args.show_json: logger.info(json.dumps(diff)) + elif self.args.show_md: + logger.info(_show_md(diff)) elif diff: logger.info(self._format(diff)) @@ -147,4 +164,10 @@ def add_parser(subparsers, parent_parser): action="store_true", default=False, ) + diff_parser.add_argument( + "--show-md", + help="Show tabulated output in the Markdown format (GFM).", + action="store_true", + default=False, + ) diff_parser.set_defaults(func=CmdDiff) diff --git a/tests/unit/command/test_diff.py b/tests/unit/command/test_diff.py index 52d033582f..c52837f7ef 100644 --- a/tests/unit/command/test_diff.py +++ b/tests/unit/command/test_diff.py @@ -3,6 +3,7 @@ import os from dvc.cli import parse_args +from dvc.command.diff import _show_md def test_default(mocker, caplog): @@ -110,3 +111,32 @@ def info(): cmd = args.func(args) assert 0 == cmd.run() assert not info() + + +def test_show_md_empty(): + assert _show_md({}) == ("| Status | Path |\n" "|----------|--------|") + + +def test_show_md(): + diff = { + "deleted": [ + {"path": "zoo", "hash": "22222"}, + {"path": os.path.join("data", ""), "hash": "XXXXXXXX.dir"}, + {"path": os.path.join("data", "foo"), "hash": "11111111"}, + {"path": os.path.join("data", "bar"), "hash": "00000000"}, + ], + "modified": [ + {"path": "file", "hash": {"old": "AAAAAAAA", "new": "BBBBBBBB"}} + ], + "added": [{"path": "file", "hash": "00000000"}], + } + assert _show_md(diff) == ( + "| Status | Path |\n" + "|----------|----------|\n" + "| added | file |\n" + "| deleted | data{sep} |\n" + "| deleted | data{sep}bar |\n" + "| deleted | data{sep}foo |\n" + "| deleted | zoo |\n" + "| modified | file |" + ).format(sep=os.path.sep)