From c63457746332762ef6f203d2e71d8f7f4bde497a Mon Sep 17 00:00:00 2001 From: David de la Iglesia Castro Date: Mon, 5 Apr 2021 13:52:29 +0200 Subject: [PATCH] =?UTF-8?q?exp=20show:=20Allow=20for=20wildcard=20patterns?= =?UTF-8?q?=20in=20include=20and=20exclude=20params/m=E2=80=A6=20(#5720)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * exp show: Allow for wildcard patterns in include and exclude params/metrics Fixes #5642 * Add test case for nested metrics/params * match full string in _filter_name * pre-commit * Update dvc/command/experiments.py Co-authored-by: Peter Rowlands (변기호) Co-authored-by: Peter Rowlands (변기호) --- dvc/command/experiments.py | 32 ++--- tests/func/experiments/test_show.py | 176 +++++++++++++++++++++++----- 2 files changed, 155 insertions(+), 53 deletions(-) diff --git a/dvc/command/experiments.py b/dvc/command/experiments.py index 90d860c8c..80b8f178b 100644 --- a/dvc/command/experiments.py +++ b/dvc/command/experiments.py @@ -3,7 +3,7 @@ import logging from collections import Counter, OrderedDict, defaultdict from collections.abc import Mapping from datetime import date, datetime -from itertools import groupby +from fnmatch import fnmatch from typing import Dict, Iterable, Optional import dvc.prompt as prompt @@ -27,32 +27,23 @@ def _filter_name(names, label, filter_strs): for filter_s in filter_strs: path, _, name = filter_s.rpartition(":") - path_filters[path].append(tuple(name.split("."))) + path_filters[path].append(name) for path, filters in path_filters.items(): if path: match_paths = [path] else: match_paths = names.keys() - for length, groups in groupby(filters, len): - for group in groups: - for match_path in match_paths: - possible_names = [ - tuple(name.split(".")) for name in names[match_path] - ] - matches = [ - name - for name in possible_names - if name[:length] == group - ] - if not matches: - name = ".".join(group) - raise InvalidArgumentError( - f"'{name}' does not match any known {label}" - ) - ret[match_path].update( - {".".join(match): None for match in matches} + for match_path in match_paths: + for f in filters: + matches = [ + name for name in names[match_path] if fnmatch(name, f) + ] + if not matches: + raise InvalidArgumentError( + f"'{f}' does not match any known {label}" ) + ret[match_path].update({match: None for match in matches}) return ret @@ -103,7 +94,6 @@ def _collect_names(all_experiments, **kwargs): for exp in experiments.values(): _update_names(metric_names, exp.get("metrics", {}).items()) _update_names(param_names, exp.get("params", {}).items()) - metric_names = _filter_names( metric_names, "metrics", diff --git a/tests/func/experiments/test_show.py b/tests/func/experiments/test_show.py index b18bb1556..b1fc539fe 100644 --- a/tests/func/experiments/test_show.py +++ b/tests/func/experiments/test_show.py @@ -5,8 +5,11 @@ from datetime import datetime import pytest from funcy import first +from dvc.exceptions import InvalidArgumentError from dvc.main import main from dvc.repo.experiments.base import ExpRefInfo +from dvc.utils.serialize import dump_yaml +from tests.func.test_repro_multistage import COPY_SCRIPT def test_show_simple(tmp_dir, scm, dvc, exp_stage): @@ -157,47 +160,156 @@ def test_show_checkpoint_branch( assert f"({branch_rev[:7]})" in cap.out -def test_show_filter(tmp_dir, scm, dvc, exp_stage, capsys): +@pytest.mark.parametrize( + "i_metrics,i_params,e_metrics,e_params,included,excluded", + [ + ( + "foo", + "foo", + None, + None, + ["foo"], + ["bar", "train/foo", "nested.foo"], + ), + ( + None, + None, + "foo", + "foo", + ["bar", "train/foo", "nested.foo"], + ["foo"], + ), + ( + "foo,bar", + "foo,bar", + None, + None, + ["foo", "bar"], + ["train/foo", "train/bar", "nested.foo", "nested.bar"], + ), + ( + "metrics.yaml:foo,bar", + "params.yaml:foo,bar", + None, + None, + ["foo", "bar"], + ["train/foo", "train/bar", "nested.foo", "nested.bar"], + ), + ( + "train/*", + "train/*", + None, + None, + ["train/foo", "train/bar"], + ["foo", "bar", "nested.foo", "nested.bar"], + ), + ( + None, + None, + "train/*", + "train/*", + ["foo", "bar", "nested.foo", "nested.bar"], + ["train/foo", "train/bar"], + ), + ( + "train/*", + "train/*", + "*foo", + "*foo", + ["train/bar"], + ["train/foo", "foo", "bar", "nested.foo", "nested.bar"], + ), + ( + "nested.*", + "nested.*", + None, + None, + ["nested.foo", "nested.bar"], + ["foo", "bar", "train/foo", "train/bar"], + ), + ( + None, + None, + "nested.*", + "nested.*", + ["foo", "bar", "train/foo", "train/bar"], + ["nested.foo", "nested.bar"], + ), + ( + "*.*", + "*.*", + "*.bar", + "*.bar", + ["nested.foo"], + ["foo", "bar", "nested.bar", "train/foo", "train/bar"], + ), + ], +) +def test_show_filter( + tmp_dir, + scm, + dvc, + capsys, + i_metrics, + i_params, + e_metrics, + e_params, + included, + excluded, +): capsys.readouterr() - assert ( - main( - [ - "exp", - "show", - "--no-pager", - "--no-timestamp", - "--include-metrics=foo", - "--include-params=foo", - ] - ) - == 0 - ) - cap = capsys.readouterr() - div = "│" if os.name == "nt" else "┃" - assert f"{div} foo {div} foo {div}" in cap.out - assert ( - main( - [ - "exp", - "show", - "--no-pager", - "--no-timestamp", - "--exclude-metrics=foo", - "--exclude-params=foo", - ] - ) - == 0 + tmp_dir.gen("copy.py", COPY_SCRIPT) + params_file = tmp_dir / "params.yaml" + params_data = { + "foo": 1, + "bar": 1, + "train/foo": 1, + "train/bar": 1, + "nested": {"foo": 1, "bar": 1}, + } + dump_yaml(params_file, params_data) + + dvc.run( + cmd="python copy.py params.yaml metrics.yaml", + metrics_no_cache=["metrics.yaml"], + params=["foo"], + name="copy-file", + deps=["copy.py"], ) + scm.add( + [ + "dvc.yaml", + "dvc.lock", + "copy.py", + "params.yaml", + "metrics.yaml", + ".gitignore", + ] + ) + scm.commit("init") + + command = ["exp", "show", "--no-pager", "--no-timestamp"] + if i_metrics is not None: + command.append(f"--include-metrics={i_metrics}") + if i_params is not None: + command.append(f"--include-params={i_params}") + if e_metrics is not None: + command.append(f"--exclude-metrics={e_metrics}") + if e_params is not None: + command.append(f"--exclude-params={e_params}") + + assert main(command) == 0 cap = capsys.readouterr() - assert f"{div} foo {div}" not in cap.out + for i in included: + assert f"{div} {i} {div}" in cap.out + for e in excluded: + assert f"{div} {e} {div}" not in cap.out def test_show_multiple_commits(tmp_dir, scm, dvc, exp_stage): - from dvc.exceptions import InvalidArgumentError - init_rev = scm.get_rev() tmp_dir.scm_gen("file", "file", "commit") next_rev = scm.get_rev()