From d6afd26beae7c4d9b4192abfb4a37506aa2186e4 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Sun, 2 May 2021 19:41:00 +0545 Subject: [PATCH] metrics diff/show: rollback to rounding fractional digits with precision (#5904) --- dvc/command/metrics.py | 2 ++ dvc/compare.py | 29 +++++++++++++++++++------ tests/func/metrics/test_diff.py | 4 ++-- tests/unit/command/test_metrics.py | 8 ++++++- tests/unit/test_compare.py | 35 +++++++++++++++++++++++++++++- 5 files changed, 67 insertions(+), 11 deletions(-) diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index 6b81c8a5c..35535ed9f 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -50,6 +50,7 @@ class CmdMetricsShow(CmdMetricsBase): all_tags=self.args.all_tags, all_commits=self.args.all_commits, precision=self.args.precision or DEFAULT_PRECISION, + round_digits=True, ) return 0 @@ -82,6 +83,7 @@ class CmdMetricsDiff(CmdMetricsBase): markdown=self.args.show_md, no_path=self.args.no_path, precision=self.args.precision or DEFAULT_PRECISION, + round_digits=True, ) return 0 diff --git a/dvc/compare.py b/dvc/compare.py index 65baa4d41..df4ee087b 100644 --- a/dvc/compare.py +++ b/dvc/compare.py @@ -172,11 +172,17 @@ class TabularData(MutableSequence[Sequence["CellT"]]): ] -def _format_field(val: Any, precision: int = None) -> str: +def _normalize_float(val: float, precision: int): + return f"{val:.{precision}g}" + + +def _format_field( + val: Any, precision: int = None, round_digits: bool = False +) -> str: def _format(_val): if isinstance(_val, float) and precision: - fmt = f"{{:.{precision}g}}" - return fmt.format(_val) + func = round if round_digits else _normalize_float + return func(_val, precision) if isinstance(_val, Mapping): return {k: _format(v) for k, v in _val.items()} if isinstance(_val, list): @@ -193,6 +199,7 @@ def diff_table( no_path: bool = False, show_changes: bool = True, precision: int = None, + round_digits: bool = False, on_empty_diff: str = None, ) -> TabularData: headers: List[str] = ["Path", title, "Old", "New", "Change"] @@ -210,9 +217,9 @@ def diff_table( [ fname, str(item), - _format_field(old_value, precision), - _format_field(new_value, precision), - _format_field(diff_value, precision), + _format_field(old_value, precision, round_digits), + _format_field(new_value, precision, round_digits), + _format_field(diff_value, precision, round_digits), ] ) @@ -236,6 +243,7 @@ def show_diff( no_path: bool = False, show_changes: bool = True, precision: int = None, + round_digits: bool = False, on_empty_diff: str = None, markdown: bool = False, ) -> None: @@ -246,6 +254,7 @@ def show_diff( no_path=no_path, show_changes=show_changes, precision=precision, + round_digits=round_digits, on_empty_diff=on_empty_diff, ) td.render(markdown=markdown) @@ -257,6 +266,7 @@ def metrics_table( all_tags: bool = False, all_commits: bool = False, precision: int = None, + round_digits: bool = False, ): from dvc.utils.diff import format_dict from dvc.utils.flatten import flatten @@ -275,7 +285,10 @@ def metrics_table( else {"": metric} ) row_data.update( - {k: _format_field(v, precision) for k, v in flattened.items()} + { + k: _format_field(v, precision, round_digits) + for k, v in flattened.items() + } ) td.row_from_dict(row_data) @@ -295,6 +308,7 @@ def show_metrics( all_tags: bool = False, all_commits: bool = False, precision: int = None, + round_digits: bool = False, ) -> None: td = metrics_table( metrics, @@ -302,5 +316,6 @@ def show_metrics( all_tags=all_tags, all_commits=all_commits, precision=precision, + round_digits=round_digits, ) td.render(markdown=markdown) diff --git a/tests/func/metrics/test_diff.py b/tests/func/metrics/test_diff.py index 7a03bdcff..5c6e847c3 100644 --- a/tests/func/metrics/test_diff.py +++ b/tests/func/metrics/test_diff.py @@ -189,8 +189,8 @@ def test_metrics_diff_cli(tmp_dir, scm, dvc, run_copy_metrics, caplog, capsys): captured = capsys.readouterr() assert captured.out == ( - "Path Metric Old New Change\n" - "m.yaml foo 1.2346 3.4568 2.2222\n" + "Path Metric Old New Change\n" + "m.yaml foo 1.23457 3.45679 2.22222\n" ) diff --git a/tests/unit/command/test_metrics.py b/tests/unit/command/test_metrics.py index 04385ec65..db94864f7 100644 --- a/tests/unit/command/test_metrics.py +++ b/tests/unit/command/test_metrics.py @@ -41,7 +41,12 @@ def test_metrics_diff(dvc, mocker, capsys): all=True, ) show_diff_mock.assert_called_once_with( - diff, title="Metric", no_path=True, precision=5, markdown=True, + diff, + title="Metric", + no_path=True, + precision=5, + markdown=True, + round_digits=True, ) @@ -125,6 +130,7 @@ def test_metrics_show(dvc, mocker): all_branches=True, all_commits=True, precision=8, + round_digits=True, ) diff --git a/tests/unit/test_compare.py b/tests/unit/test_compare.py index 55caeb155..b9645d518 100644 --- a/tests/unit/test_compare.py +++ b/tests/unit/test_compare.py @@ -66,7 +66,7 @@ def test_diff_table_precision(): "a.b.c": {"old": 1.1234, "new": 2.2345, "diff": 3.3456} } } - td = diff_table(diff, title="Metric", precision=3,) + td = diff_table(diff, title="Metric", precision=3) assert td.as_dict() == [ { "Path": "metrics.json", @@ -78,6 +78,24 @@ def test_diff_table_precision(): ] +def test_diff_table_rounding(): + diff = { + "metrics.json": { + "a.b.c": {"old": 1.1234, "new": 2.2345, "diff": 3.3456} + } + } + td = diff_table(diff, title="Metric", precision=3, round_digits=True) + assert td.as_dict() == [ + { + "Path": "metrics.json", + "Metric": "a.b.c", + "Old": "1.123", + "New": "2.235", + "Change": "3.346", + } + ] + + @pytest.mark.parametrize( "extra, expected", [({"on_empty_diff": "no diff"}, "no diff"), ({}, "-")] ) @@ -194,6 +212,7 @@ def test_diff_mocked(mocker, markdown): precision=None, on_empty_diff=None, show_changes=True, + round_digits=False, ) ret.render.assert_called_once_with(markdown=markdown) @@ -381,6 +400,19 @@ def test_metrics_show_precision(): } ] + td = metrics_table( + metrics, all_branches=True, precision=4, round_digits=True + ) + assert td.as_dict() == [ + { + "Revision": "branch_1", + "Path": "metrics.json", + "a": "1.0988", + "b.ad": "1.5343", + "b.bc": "2.9877", + } + ] + td = metrics_table(metrics, all_branches=True, precision=7) assert td.as_dict() == [ { @@ -406,6 +438,7 @@ def test_metrics_show_mocked(mocker, markdown): all_tags=False, all_commits=False, precision=None, + round_digits=False, ) ret.render.assert_called_once_with(markdown=markdown)