From 725b02dadf79f8c9ae796cf01bbe030368d0d3e2 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 21:45:22 +0100 Subject: [PATCH 01/25] Add keep_selected parameter, and corresponding code to keep only the selected exps (and remove all the other ones) --- dvc/repo/experiments/remove.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index ef6ee086ab..e74a13f904 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -30,6 +30,7 @@ def remove( # noqa: C901, PLR0912 num: int = 1, queue: bool = False, git_remote: Optional[str] = None, + keep_selected: bool = False, # keep the experiments instead of removing them ) -> list[str]: removed: list[str] = [] if not any([exp_names, queue, all_commits, rev]): @@ -43,6 +44,30 @@ def remove( # noqa: C901, PLR0912 exp_ref_list: list[ExpRefInfo] = [] queue_entry_list: list[QueueEntry] = [] + + if keep_selected: + # In keep_selected mode, identify all experiments and remove the unselected ones + all_exp_refs = exp_refs(repo.scm, git_remote) + + if exp_names: + selected_exp_names = set(exp_names) if isinstance(exp_names, list) else {exp_names} + elif rev: + selected_exp_names = set( + _resolve_exp_by_baseline(repo, [rev] if isinstance(rev, str) else rev, num, git_remote).keys() + ) + else: + selected_exp_names = set() + + # Identify experiments to remove: all experiments - selected experiments + unselected_exp_refs = [ref for ref in all_exp_refs if ref.name not in selected_exp_names] + removed = [ref.name for ref in unselected_exp_refs] + + # Remove the unselected experiments + if unselected_exp_refs: + _remove_commited_exps(repo.scm, unselected_exp_refs, git_remote) + + return removed + if exp_names: results: dict[str, ExpRefAndQueueEntry] = ( celery_queue.get_ref_and_entry_by_names(exp_names, git_remote) @@ -83,6 +108,7 @@ def remove( # noqa: C901, PLR0912 removed_refs = [str(r) for r in exp_ref_list] notify_refs_to_studio(repo, git_remote, removed=removed_refs) + return removed From e47cc727272780a3770fa76c494c949e9f4582d0 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 22:17:33 +0100 Subject: [PATCH 02/25] test keep_selected_by_name --- tests/func/experiments/test_remove.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 60d653d1e8..6264f67293 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -179,3 +179,29 @@ def test_remove_multi_rev(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(baseline_exp_ref)) is None assert scm.get_ref(str(new_exp_ref)) is None + +def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): + baseline = scm.get_rev() + + # Setup: Run experiments + results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") + exp1_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + exp2_ref = first(exp_refs_by_rev(scm, first(results))) + results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") + exp3_ref = first(exp_refs_by_rev(scm, first(results))) + + # Ensure experiments exist + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None + + # Keep "exp2" and remove others + removed = dvc.experiments.remove(exp_names=["exp2"], keep_selected=True) + assert removed == ["exp1", "exp3"] + + # Check remaining experiments + assert scm.get_ref(str(exp1_ref)) is None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is None \ No newline at end of file From df6a7ed69c91341fe43bc70944624302b5600330 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 22:22:54 +0100 Subject: [PATCH 03/25] test keep_selected_by_rev --- tests/func/experiments/test_remove.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 6264f67293..de601c585a 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -204,4 +204,27 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): # Check remaining experiments assert scm.get_ref(str(exp1_ref)) is None assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is None \ No newline at end of file + assert scm.get_ref(str(exp3_ref)) is None + +def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): + # Setup: Run experiments and commit + baseline = scm.get_rev() + results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") + exp1_ref = first(exp_refs_by_rev(scm, first(results))) + scm.commit("commit1") + + new_results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + exp2_ref = first(exp_refs_by_rev(scm, first(new_results))) + new_rev = scm.get_rev() + + # Ensure experiments exist + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + + # Keep the experiment from the new revision + removed = dvc.experiments.remove(rev=new_rev, num=1, keep_selected=True) + assert removed == ["exp1"] + + # Check remaining experiments + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp1_ref)) is None \ No newline at end of file From df2a1eebd6007f30465402b5368a2c3073ccf6a8 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 22:25:26 +0100 Subject: [PATCH 04/25] test keep_selected multiple, by name --- tests/func/experiments/test_remove.py | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index de601c585a..385088fa8a 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -189,6 +189,7 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") exp2_ref = first(exp_refs_by_rev(scm, first(results))) + results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") exp3_ref = first(exp_refs_by_rev(scm, first(results))) @@ -206,6 +207,33 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is None +def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): + baseline = scm.get_rev() + + # Setup: Run experiments + results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") + exp1_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + exp2_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") + exp3_ref = first(exp_refs_by_rev(scm, first(results))) + + # Ensure experiments exist + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None + + # Keep "exp1" and "exp2" and remove "exp3" + removed = dvc.experiments.remove(exp_names=["exp1","exp2"], keep_selected=True) + assert removed == ["exp3"] + + # Check remaining experiments + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is None + def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit baseline = scm.get_rev() From 4f32f201cd2050cca5f6d31ba72df1496e051018 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 22:27:37 +0100 Subject: [PATCH 05/25] test keep all by name --- tests/func/experiments/test_remove.py | 29 +++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 385088fa8a..e8789109ef 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -181,7 +181,6 @@ def test_remove_multi_rev(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(new_exp_ref)) is None def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): - baseline = scm.get_rev() # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") @@ -208,7 +207,6 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is None def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): - baseline = scm.get_rev() # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") @@ -234,6 +232,33 @@ def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is None +def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): + + + # Setup: Run experiments + results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") + exp1_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + exp2_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") + exp3_ref = first(exp_refs_by_rev(scm, first(results))) + + # Ensure experiments exist + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None + + # Keep "exp1" and "exp2" and remove "exp3" + removed = dvc.experiments.remove(exp_names=["exp1","exp2", "exp3"], keep_selected=True) + assert removed == [] + + # Check remaining experiments + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None + def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit baseline = scm.get_rev() From f913ffb0b6c72fbf0d49434ea8f624f9db950b8c Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 22:36:01 +0100 Subject: [PATCH 06/25] test keep by rev, with num=2 --- tests/func/experiments/test_remove.py | 59 ++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index e8789109ef..7adac42907 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -233,8 +233,6 @@ def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is None def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): - - # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") exp1_ref = first(exp_refs_by_rev(scm, first(results))) @@ -259,6 +257,31 @@ def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is not None +def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): + # Setup: Run experiments + results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") + exp1_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + exp2_ref = first(exp_refs_by_rev(scm, first(results))) + + results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") + exp3_ref = first(exp_refs_by_rev(scm, first(results))) + + # Ensure experiments exist + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None + + # Keep "exp1" and "exp2" and remove "exp3" + removed = dvc.experiments.remove(exp_names=["nonexistent"], keep_selected=True) + assert removed == ["exp1", "exp2", "exp3"] + + # Check remaining experiments + assert scm.get_ref(str(exp1_ref)) is None + assert scm.get_ref(str(exp2_ref)) is None + assert scm.get_ref(str(exp3_ref)) is None + def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit baseline = scm.get_rev() @@ -280,4 +303,36 @@ def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): # Check remaining experiments assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp1_ref)) is None + +def test_keep_selected_by_rev_multiple(tmp_dir, scm, dvc, exp_stage): + # Setup: Run experiments and commit + baseline = scm.get_rev() + exp1_rev = scm.get_rev() + results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") + exp1_ref = first(exp_refs_by_rev(scm, first(results))) + scm.commit("commit1") + + exp2_rev = scm.get_rev() + results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + exp2_ref = first(exp_refs_by_rev(scm, first(results))) + scm.commit("commit2") + + exp3_rev = scm.get_rev() + results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") + exp3_ref = first(exp_refs_by_rev(scm, first(results))) + scm.commit("commit3") + + # Ensure experiments exist + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None + + # Keep the last 2, remove first + removed = dvc.experiments.remove(rev=exp3_rev, num=2, keep_selected=True) + assert removed == ["exp1"] + + # Check remaining experiments + assert scm.get_ref(str(exp3_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp1_ref)) is None \ No newline at end of file From c3a46da1d9d6513742449897f195bc29e6c60314 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 22:42:27 +0100 Subject: [PATCH 07/25] added option to cli --- dvc/commands/experiments/__init__.py | 8 +++++++- dvc/commands/experiments/remove.py | 4 +++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index 6b29bfacb5..952557f1f3 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -57,7 +57,13 @@ def add_parser(subparsers, parent_parser): cmd.add_parser(experiments_subparsers, parent_parser) hide_subparsers_from_help(experiments_subparsers) - +def add_keep_selection_flag(experiments_subcmd_parser): + experiments_subcmd_parser.add_argument( + "--keep", + action="store_true", + default=False, + help="Keep the selected experiments instead of removing them (use it with `--rev` and `--num` or with experiment names).", + ) def add_rev_selection_flags( experiments_subcmd_parser, command: str, default: bool = True ): diff --git a/dvc/commands/experiments/remove.py b/dvc/commands/experiments/remove.py index f7c48d73ac..c5525388ba 100644 --- a/dvc/commands/experiments/remove.py +++ b/dvc/commands/experiments/remove.py @@ -34,6 +34,7 @@ def run(self): num=self.args.num, queue=self.args.queue, git_remote=self.args.git_remote, + keep_selected=self.args.keep ) if removed: ui.write(f"Removed experiments: {humanize.join(map(repr, removed))}") @@ -44,7 +45,7 @@ def run(self): def add_parser(experiments_subparsers, parent_parser): - from . import add_rev_selection_flags + from . import add_rev_selection_flags, add_keep_selection_flag EXPERIMENTS_REMOVE_HELP = "Remove experiments." experiments_remove_parser = experiments_subparsers.add_parser( @@ -57,6 +58,7 @@ def add_parser(experiments_subparsers, parent_parser): ) remove_group = experiments_remove_parser.add_mutually_exclusive_group() add_rev_selection_flags(experiments_remove_parser, "Remove", False) + add_keep_selection_flag(experiments_remove_parser) remove_group.add_argument( "--queue", action="store_true", help="Remove all queued experiments." ) From 4e84b2d2d426f7bc38c9714f1adff642293545b3 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 23:18:33 +0100 Subject: [PATCH 08/25] refactoring to meet pr needs --- dvc/commands/experiments/__init__.py | 3 ++- dvc/repo/experiments/remove.py | 21 ++++++++++++--------- tests/func/experiments/test_remove.py | 4 ---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index 952557f1f3..656546c513 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -62,7 +62,8 @@ def add_keep_selection_flag(experiments_subcmd_parser): "--keep", action="store_true", default=False, - help="Keep the selected experiments instead of removing them (use it with `--rev` and `--num` or with experiment names).", + help="Keep the selected experiments instead of removing them " + "(use it with `--rev`' and `--num` or with experiment names)." ) def add_rev_selection_flags( experiments_subcmd_parser, command: str, default: bool = True diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index e74a13f904..720d0f6c21 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -48,15 +48,7 @@ def remove( # noqa: C901, PLR0912 if keep_selected: # In keep_selected mode, identify all experiments and remove the unselected ones all_exp_refs = exp_refs(repo.scm, git_remote) - - if exp_names: - selected_exp_names = set(exp_names) if isinstance(exp_names, list) else {exp_names} - elif rev: - selected_exp_names = set( - _resolve_exp_by_baseline(repo, [rev] if isinstance(rev, str) else rev, num, git_remote).keys() - ) - else: - selected_exp_names = set() + selected_exp_names = resolve_selected_exp_names(exp_names, git_remote, num, repo, rev) # Identify experiments to remove: all experiments - selected experiments unselected_exp_refs = [ref for ref in all_exp_refs if ref.name not in selected_exp_names] @@ -112,6 +104,17 @@ def remove( # noqa: C901, PLR0912 return removed +def resolve_selected_exp_names(exp_names, git_remote, num, repo, rev): + selected_exp_names = set() + if exp_names: + selected_exp_names = set(exp_names) if isinstance(exp_names, list) else {exp_names} + elif rev: + selected_exp_names = set( + _resolve_exp_by_baseline(repo, [rev] if isinstance(rev, str) else rev, num, git_remote).keys() + ) + return selected_exp_names + + def _resolve_exp_by_baseline( repo: "Repo", rev: list[str], diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 7adac42907..109ccf5830 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -284,7 +284,6 @@ def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit - baseline = scm.get_rev() results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") exp1_ref = first(exp_refs_by_rev(scm, first(results))) scm.commit("commit1") @@ -307,13 +306,10 @@ def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): def test_keep_selected_by_rev_multiple(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit - baseline = scm.get_rev() - exp1_rev = scm.get_rev() results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") exp1_ref = first(exp_refs_by_rev(scm, first(results))) scm.commit("commit1") - exp2_rev = scm.get_rev() results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") exp2_ref = first(exp_refs_by_rev(scm, first(results))) scm.commit("commit2") From 7f050eddcde1eee048c24aca66ffbc6ffea2cea7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 23 Nov 2024 22:27:32 +0000 Subject: [PATCH 09/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/commands/experiments/__init__.py | 5 ++++- dvc/commands/experiments/remove.py | 4 ++-- dvc/repo/experiments/remove.py | 18 +++++++++++++----- tests/func/experiments/test_remove.py | 20 ++++++++++++++------ 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index 656546c513..780e6e18a8 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -57,14 +57,17 @@ def add_parser(subparsers, parent_parser): cmd.add_parser(experiments_subparsers, parent_parser) hide_subparsers_from_help(experiments_subparsers) + def add_keep_selection_flag(experiments_subcmd_parser): experiments_subcmd_parser.add_argument( "--keep", action="store_true", default=False, help="Keep the selected experiments instead of removing them " - "(use it with `--rev`' and `--num` or with experiment names)." + "(use it with `--rev`' and `--num` or with experiment names).", ) + + def add_rev_selection_flags( experiments_subcmd_parser, command: str, default: bool = True ): diff --git a/dvc/commands/experiments/remove.py b/dvc/commands/experiments/remove.py index c5525388ba..7f63ece2b0 100644 --- a/dvc/commands/experiments/remove.py +++ b/dvc/commands/experiments/remove.py @@ -34,7 +34,7 @@ def run(self): num=self.args.num, queue=self.args.queue, git_remote=self.args.git_remote, - keep_selected=self.args.keep + keep_selected=self.args.keep, ) if removed: ui.write(f"Removed experiments: {humanize.join(map(repr, removed))}") @@ -45,7 +45,7 @@ def run(self): def add_parser(experiments_subparsers, parent_parser): - from . import add_rev_selection_flags, add_keep_selection_flag + from . import add_keep_selection_flag, add_rev_selection_flags EXPERIMENTS_REMOVE_HELP = "Remove experiments." experiments_remove_parser = experiments_subparsers.add_parser( diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index 720d0f6c21..dfc0884170 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -30,7 +30,7 @@ def remove( # noqa: C901, PLR0912 num: int = 1, queue: bool = False, git_remote: Optional[str] = None, - keep_selected: bool = False, # keep the experiments instead of removing them + keep_selected: bool = False, # keep the experiments instead of removing them ) -> list[str]: removed: list[str] = [] if not any([exp_names, queue, all_commits, rev]): @@ -48,10 +48,14 @@ def remove( # noqa: C901, PLR0912 if keep_selected: # In keep_selected mode, identify all experiments and remove the unselected ones all_exp_refs = exp_refs(repo.scm, git_remote) - selected_exp_names = resolve_selected_exp_names(exp_names, git_remote, num, repo, rev) + selected_exp_names = resolve_selected_exp_names( + exp_names, git_remote, num, repo, rev + ) # Identify experiments to remove: all experiments - selected experiments - unselected_exp_refs = [ref for ref in all_exp_refs if ref.name not in selected_exp_names] + unselected_exp_refs = [ + ref for ref in all_exp_refs if ref.name not in selected_exp_names + ] removed = [ref.name for ref in unselected_exp_refs] # Remove the unselected experiments @@ -107,10 +111,14 @@ def remove( # noqa: C901, PLR0912 def resolve_selected_exp_names(exp_names, git_remote, num, repo, rev): selected_exp_names = set() if exp_names: - selected_exp_names = set(exp_names) if isinstance(exp_names, list) else {exp_names} + selected_exp_names = ( + set(exp_names) if isinstance(exp_names, list) else {exp_names} + ) elif rev: selected_exp_names = set( - _resolve_exp_by_baseline(repo, [rev] if isinstance(rev, str) else rev, num, git_remote).keys() + _resolve_exp_by_baseline( + repo, [rev] if isinstance(rev, str) else rev, num, git_remote + ).keys() ) return selected_exp_names diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 109ccf5830..06f0aa84f4 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -180,8 +180,8 @@ def test_remove_multi_rev(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(baseline_exp_ref)) is None assert scm.get_ref(str(new_exp_ref)) is None -def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): +def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") exp1_ref = first(exp_refs_by_rev(scm, first(results))) @@ -206,8 +206,8 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is None -def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): +def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") exp1_ref = first(exp_refs_by_rev(scm, first(results))) @@ -224,7 +224,7 @@ def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["exp1","exp2"], keep_selected=True) + removed = dvc.experiments.remove(exp_names=["exp1", "exp2"], keep_selected=True) assert removed == ["exp3"] # Check remaining experiments @@ -232,6 +232,7 @@ def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is None + def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") @@ -249,7 +250,9 @@ def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["exp1","exp2", "exp3"], keep_selected=True) + removed = dvc.experiments.remove( + exp_names=["exp1", "exp2", "exp3"], keep_selected=True + ) assert removed == [] # Check remaining experiments @@ -257,6 +260,7 @@ def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is not None + def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") @@ -282,13 +286,16 @@ def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is None assert scm.get_ref(str(exp3_ref)) is None + def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") exp1_ref = first(exp_refs_by_rev(scm, first(results))) scm.commit("commit1") - new_results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") + new_results = dvc.experiments.run( + exp_stage.addressing, params=["foo=2"], name="exp2" + ) exp2_ref = first(exp_refs_by_rev(scm, first(new_results))) new_rev = scm.get_rev() @@ -304,6 +311,7 @@ def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp1_ref)) is None + def test_keep_selected_by_rev_multiple(tmp_dir, scm, dvc, exp_stage): # Setup: Run experiments and commit results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") @@ -331,4 +339,4 @@ def test_keep_selected_by_rev_multiple(tmp_dir, scm, dvc, exp_stage): # Check remaining experiments assert scm.get_ref(str(exp3_ref)) is not None assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp1_ref)) is None \ No newline at end of file + assert scm.get_ref(str(exp1_ref)) is None From 36a0e2b4393eee708045802e0288fe0aa16eeb82 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sat, 23 Nov 2024 23:41:02 +0100 Subject: [PATCH 10/25] fixed test_experiments to add keep_selected=False to remove tests --- tests/unit/command/test_experiments.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/command/test_experiments.py b/tests/unit/command/test_experiments.py index 686d414b92..dcc9e20cc2 100644 --- a/tests/unit/command/test_experiments.py +++ b/tests/unit/command/test_experiments.py @@ -384,6 +384,7 @@ def test_experiments_remove_flag(dvc, scm, mocker, capsys, caplog): num=2, queue=False, git_remote="myremote", + keep_selected=False ) @@ -410,6 +411,7 @@ def test_experiments_remove_special(dvc, scm, mocker, capsys, caplog): num=1, queue=False, git_remote="myremote", + keep_selected=False ) From 87abac15981f069d0ad173b862f1ca6d6b28a290 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 24 Nov 2024 08:39:47 +0100 Subject: [PATCH 11/25] rename parameter to match cli option --- dvc/commands/experiments/remove.py | 2 +- dvc/repo/experiments/remove.py | 4 ++-- tests/func/experiments/test_remove.py | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dvc/commands/experiments/remove.py b/dvc/commands/experiments/remove.py index 7f63ece2b0..ecb6541c07 100644 --- a/dvc/commands/experiments/remove.py +++ b/dvc/commands/experiments/remove.py @@ -34,7 +34,7 @@ def run(self): num=self.args.num, queue=self.args.queue, git_remote=self.args.git_remote, - keep_selected=self.args.keep, + keep=self.args.keep, ) if removed: ui.write(f"Removed experiments: {humanize.join(map(repr, removed))}") diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index dfc0884170..936e4fd94c 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -30,7 +30,7 @@ def remove( # noqa: C901, PLR0912 num: int = 1, queue: bool = False, git_remote: Optional[str] = None, - keep_selected: bool = False, # keep the experiments instead of removing them + keep: bool = False, ) -> list[str]: removed: list[str] = [] if not any([exp_names, queue, all_commits, rev]): @@ -45,7 +45,7 @@ def remove( # noqa: C901, PLR0912 exp_ref_list: list[ExpRefInfo] = [] queue_entry_list: list[QueueEntry] = [] - if keep_selected: + if keep: # In keep_selected mode, identify all experiments and remove the unselected ones all_exp_refs = exp_refs(repo.scm, git_remote) selected_exp_names = resolve_selected_exp_names( diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 06f0aa84f4..233b4ab323 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -198,7 +198,7 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep "exp2" and remove others - removed = dvc.experiments.remove(exp_names=["exp2"], keep_selected=True) + removed = dvc.experiments.remove(exp_names=["exp2"], keep=True) assert removed == ["exp1", "exp3"] # Check remaining experiments @@ -224,7 +224,7 @@ def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["exp1", "exp2"], keep_selected=True) + removed = dvc.experiments.remove(exp_names=["exp1", "exp2"], keep=True) assert removed == ["exp3"] # Check remaining experiments @@ -251,7 +251,7 @@ def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): # Keep "exp1" and "exp2" and remove "exp3" removed = dvc.experiments.remove( - exp_names=["exp1", "exp2", "exp3"], keep_selected=True + exp_names=["exp1", "exp2", "exp3"], keep=True ) assert removed == [] @@ -278,7 +278,7 @@ def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["nonexistent"], keep_selected=True) + removed = dvc.experiments.remove(exp_names=["nonexistent"], keep=True) assert removed == ["exp1", "exp2", "exp3"] # Check remaining experiments @@ -304,7 +304,7 @@ def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None # Keep the experiment from the new revision - removed = dvc.experiments.remove(rev=new_rev, num=1, keep_selected=True) + removed = dvc.experiments.remove(rev=new_rev, num=1, keep=True) assert removed == ["exp1"] # Check remaining experiments @@ -333,7 +333,7 @@ def test_keep_selected_by_rev_multiple(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep the last 2, remove first - removed = dvc.experiments.remove(rev=exp3_rev, num=2, keep_selected=True) + removed = dvc.experiments.remove(rev=exp3_rev, num=2, keep=True) assert removed == ["exp1"] # Check remaining experiments From ca279b91f055c450de27e5b5a931f12e1b51a6b5 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 24 Nov 2024 09:17:07 +0100 Subject: [PATCH 12/25] follow the normal path, then invert the selection before removing --- dvc/repo/experiments/remove.py | 35 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index 936e4fd94c..237c166ba3 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -30,7 +30,7 @@ def remove( # noqa: C901, PLR0912 num: int = 1, queue: bool = False, git_remote: Optional[str] = None, - keep: bool = False, + keep: bool = False, ) -> list[str]: removed: list[str] = [] if not any([exp_names, queue, all_commits, rev]): @@ -45,25 +45,6 @@ def remove( # noqa: C901, PLR0912 exp_ref_list: list[ExpRefInfo] = [] queue_entry_list: list[QueueEntry] = [] - if keep: - # In keep_selected mode, identify all experiments and remove the unselected ones - all_exp_refs = exp_refs(repo.scm, git_remote) - selected_exp_names = resolve_selected_exp_names( - exp_names, git_remote, num, repo, rev - ) - - # Identify experiments to remove: all experiments - selected experiments - unselected_exp_refs = [ - ref for ref in all_exp_refs if ref.name not in selected_exp_names - ] - removed = [ref.name for ref in unselected_exp_refs] - - # Remove the unselected experiments - if unselected_exp_refs: - _remove_commited_exps(repo.scm, unselected_exp_refs, git_remote) - - return removed - if exp_names: results: dict[str, ExpRefAndQueueEntry] = ( celery_queue.get_ref_and_entry_by_names(exp_names, git_remote) @@ -91,6 +72,20 @@ def remove( # noqa: C901, PLR0912 exp_ref_list.extend(exp_refs(repo.scm, git_remote)) removed = [ref.name for ref in exp_ref_list] + if keep: + all_exp_refs = exp_refs(repo.scm, git_remote) # Get all experiments + selected_exp_names = {ref.name for ref in exp_ref_list} # Current selection + exp_ref_list = [ref for ref in all_exp_refs if ref.name not in selected_exp_names] + + # Handle queued experiments + all_queue_entries = list(celery_queue.iter_queued()) + selected_queue_entries = {entry.name for entry in queue_entry_list} + queue_entry_list = [ + entry for entry in all_queue_entries if entry.name not in selected_queue_entries + ] + + removed = [ref.name for ref in exp_ref_list] + [entry.name for entry in queue_entry_list] + if exp_ref_list: _remove_commited_exps(repo.scm, exp_ref_list, git_remote) From a90350760fc99055f9f2bf87e96d096f8e000605 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 24 Nov 2024 09:19:01 +0100 Subject: [PATCH 13/25] fixed tests for list ordering + fixed test with non existent name, it didn't make sense to delete everything if an exp name did not exist --- tests/func/experiments/test_remove.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 233b4ab323..ae48399a69 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -199,7 +199,7 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): # Keep "exp2" and remove others removed = dvc.experiments.remove(exp_names=["exp2"], keep=True) - assert removed == ["exp1", "exp3"] + assert sorted(removed) == ["exp1", "exp3"] # Check remaining experiments assert scm.get_ref(str(exp1_ref)) is None @@ -277,14 +277,15 @@ def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp2_ref)) is not None assert scm.get_ref(str(exp3_ref)) is not None - # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["nonexistent"], keep=True) - assert removed == ["exp1", "exp2", "exp3"] + # non existent name should raise an error + with pytest.raises(UnresolvedExpNamesError): + dvc.experiments.remove(exp_names=["nonexistent"], keep=True) - # Check remaining experiments - assert scm.get_ref(str(exp1_ref)) is None - assert scm.get_ref(str(exp2_ref)) is None - assert scm.get_ref(str(exp3_ref)) is None + + # Check nothing has been deleted + assert scm.get_ref(str(exp1_ref)) is not None + assert scm.get_ref(str(exp2_ref)) is not None + assert scm.get_ref(str(exp3_ref)) is not None def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): From cb43ca36d846f5f9e1dbb3ac339f44353b297519 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 24 Nov 2024 13:44:07 +0100 Subject: [PATCH 14/25] changed cli option comment --- dvc/commands/experiments/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index 780e6e18a8..5bdc54cf79 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -64,7 +64,6 @@ def add_keep_selection_flag(experiments_subcmd_parser): action="store_true", default=False, help="Keep the selected experiments instead of removing them " - "(use it with `--rev`' and `--num` or with experiment names).", ) From 3184250f9154f6f3a3b6f88807ba2512a71723f1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 24 Nov 2024 12:44:22 +0000 Subject: [PATCH 15/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/commands/experiments/__init__.py | 2 +- dvc/repo/experiments/remove.py | 12 +++++++++--- tests/func/experiments/test_remove.py | 5 +---- tests/unit/command/test_experiments.py | 4 ++-- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index 5bdc54cf79..d0f1b05eff 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -63,7 +63,7 @@ def add_keep_selection_flag(experiments_subcmd_parser): "--keep", action="store_true", default=False, - help="Keep the selected experiments instead of removing them " + help="Keep the selected experiments instead of removing them ", ) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index 237c166ba3..7214616f82 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -75,16 +75,22 @@ def remove( # noqa: C901, PLR0912 if keep: all_exp_refs = exp_refs(repo.scm, git_remote) # Get all experiments selected_exp_names = {ref.name for ref in exp_ref_list} # Current selection - exp_ref_list = [ref for ref in all_exp_refs if ref.name not in selected_exp_names] + exp_ref_list = [ + ref for ref in all_exp_refs if ref.name not in selected_exp_names + ] # Handle queued experiments all_queue_entries = list(celery_queue.iter_queued()) selected_queue_entries = {entry.name for entry in queue_entry_list} queue_entry_list = [ - entry for entry in all_queue_entries if entry.name not in selected_queue_entries + entry + for entry in all_queue_entries + if entry.name not in selected_queue_entries ] - removed = [ref.name for ref in exp_ref_list] + [entry.name for entry in queue_entry_list] + removed = [ref.name for ref in exp_ref_list] + [ + entry.name for entry in queue_entry_list + ] if exp_ref_list: _remove_commited_exps(repo.scm, exp_ref_list, git_remote) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index ae48399a69..d0b721d844 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -250,9 +250,7 @@ def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(exp3_ref)) is not None # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove( - exp_names=["exp1", "exp2", "exp3"], keep=True - ) + removed = dvc.experiments.remove(exp_names=["exp1", "exp2", "exp3"], keep=True) assert removed == [] # Check remaining experiments @@ -281,7 +279,6 @@ def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): with pytest.raises(UnresolvedExpNamesError): dvc.experiments.remove(exp_names=["nonexistent"], keep=True) - # Check nothing has been deleted assert scm.get_ref(str(exp1_ref)) is not None assert scm.get_ref(str(exp2_ref)) is not None diff --git a/tests/unit/command/test_experiments.py b/tests/unit/command/test_experiments.py index dcc9e20cc2..b2089f0818 100644 --- a/tests/unit/command/test_experiments.py +++ b/tests/unit/command/test_experiments.py @@ -384,7 +384,7 @@ def test_experiments_remove_flag(dvc, scm, mocker, capsys, caplog): num=2, queue=False, git_remote="myremote", - keep_selected=False + keep_selected=False, ) @@ -411,7 +411,7 @@ def test_experiments_remove_special(dvc, scm, mocker, capsys, caplog): num=1, queue=False, git_remote="myremote", - keep_selected=False + keep_selected=False, ) From dea54cb5ca14c83fe99ca177083e929087fb8f2a Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 24 Nov 2024 14:20:03 +0100 Subject: [PATCH 16/25] fixed typing issue --- dvc/repo/experiments/remove.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index 7214616f82..a75a168592 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -89,7 +89,7 @@ def remove( # noqa: C901, PLR0912 ] removed = [ref.name for ref in exp_ref_list] + [ - entry.name for entry in queue_entry_list + entry.name for entry in queue_entry_list if entry.name ] if exp_ref_list: From 421c3f0aabd14e7c65b28fbb8c790d830b53f08a Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 24 Nov 2024 14:38:08 +0100 Subject: [PATCH 17/25] updated parameter name --- tests/unit/command/test_experiments.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/command/test_experiments.py b/tests/unit/command/test_experiments.py index b2089f0818..5831eacad4 100644 --- a/tests/unit/command/test_experiments.py +++ b/tests/unit/command/test_experiments.py @@ -384,7 +384,7 @@ def test_experiments_remove_flag(dvc, scm, mocker, capsys, caplog): num=2, queue=False, git_remote="myremote", - keep_selected=False, + keep=False, ) @@ -411,7 +411,7 @@ def test_experiments_remove_special(dvc, scm, mocker, capsys, caplog): num=1, queue=False, git_remote="myremote", - keep_selected=False, + keep=False, ) From 0a76842fbebc72270f2181abf2c945b2442ed64b Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Tue, 26 Nov 2024 22:22:47 +0100 Subject: [PATCH 18/25] removed handling queued experiments (since --queue would remove them all) --- dvc/commands/experiments/__init__.py | 3 ++- dvc/repo/experiments/remove.py | 13 +------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index d0f1b05eff..b199f2cb88 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -63,7 +63,8 @@ def add_keep_selection_flag(experiments_subcmd_parser): "--keep", action="store_true", default=False, - help="Keep the selected experiments instead of removing them ", + help="Keep the selected (committed, not queued) experiments " + "instead of removing them.", ) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index a75a168592..a35b1273f8 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -79,18 +79,7 @@ def remove( # noqa: C901, PLR0912 ref for ref in all_exp_refs if ref.name not in selected_exp_names ] - # Handle queued experiments - all_queue_entries = list(celery_queue.iter_queued()) - selected_queue_entries = {entry.name for entry in queue_entry_list} - queue_entry_list = [ - entry - for entry in all_queue_entries - if entry.name not in selected_queue_entries - ] - - removed = [ref.name for ref in exp_ref_list] + [ - entry.name for entry in queue_entry_list if entry.name - ] + removed = [ref.name for ref in exp_ref_list] if exp_ref_list: _remove_commited_exps(repo.scm, exp_ref_list, git_remote) From 783b67a73083dd3fca6d99e04ead8ad90b65971f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 26 Nov 2024 21:24:48 +0000 Subject: [PATCH 19/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/commands/experiments/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index b199f2cb88..fc949e9e49 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -64,7 +64,7 @@ def add_keep_selection_flag(experiments_subcmd_parser): action="store_true", default=False, help="Keep the selected (committed, not queued) experiments " - "instead of removing them.", + "instead of removing them.", ) From b83108a79f7a14c5f2911a3a08ed36d37f28d684 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Wed, 27 Nov 2024 22:27:26 +0100 Subject: [PATCH 20/25] code simplification, added __eq__ and __hash__ to be able to compare ExpRefs, updated and parametrized tests. --- dvc/repo/experiments/refs.py | 9 ++ dvc/repo/experiments/remove.py | 25 +--- tests/func/experiments/test_remove.py | 192 ++++++++------------------ 3 files changed, 66 insertions(+), 160 deletions(-) diff --git a/dvc/repo/experiments/refs.py b/dvc/repo/experiments/refs.py index 5497a25c77..ef7b281f62 100644 --- a/dvc/repo/experiments/refs.py +++ b/dvc/repo/experiments/refs.py @@ -67,3 +67,12 @@ def from_ref(cls, ref: str): baseline_sha = parts[2] + parts[3] name = parts[4] if len(parts) == 5 else None return cls(baseline_sha, name) + + def __eq__(self, other): + if not isinstance(other, ExpRefInfo): + return False + + return self.baseline_sha == other.baseline_sha and self.name == other.name + + def __hash__(self): + return hash((self.baseline_sha, self.name)) \ No newline at end of file diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index a35b1273f8..dcdcdd398f 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -33,6 +33,7 @@ def remove( # noqa: C901, PLR0912 keep: bool = False, ) -> list[str]: removed: list[str] = [] + if not any([exp_names, queue, all_commits, rev]): return removed celery_queue: LocalCeleryQueue = repo.experiments.celery_queue @@ -72,13 +73,9 @@ def remove( # noqa: C901, PLR0912 exp_ref_list.extend(exp_refs(repo.scm, git_remote)) removed = [ref.name for ref in exp_ref_list] - if keep: - all_exp_refs = exp_refs(repo.scm, git_remote) # Get all experiments - selected_exp_names = {ref.name for ref in exp_ref_list} # Current selection - exp_ref_list = [ - ref for ref in all_exp_refs if ref.name not in selected_exp_names - ] + if keep: + exp_ref_list = list(set(exp_refs(repo.scm, git_remote)) - set(exp_ref_list)) removed = [ref.name for ref in exp_ref_list] if exp_ref_list: @@ -97,22 +94,6 @@ def remove( # noqa: C901, PLR0912 return removed - -def resolve_selected_exp_names(exp_names, git_remote, num, repo, rev): - selected_exp_names = set() - if exp_names: - selected_exp_names = ( - set(exp_names) if isinstance(exp_names, list) else {exp_names} - ) - elif rev: - selected_exp_names = set( - _resolve_exp_by_baseline( - repo, [rev] if isinstance(rev, str) else rev, num, git_remote - ).keys() - ) - return selected_exp_names - - def _resolve_exp_by_baseline( repo: "Repo", rev: list[str], diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index d0b721d844..cc31324227 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -181,160 +181,76 @@ def test_remove_multi_rev(tmp_dir, scm, dvc, exp_stage): assert scm.get_ref(str(new_exp_ref)) is None -def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage): +@pytest.mark.parametrize( + "keep, expected_removed", + [ + [["exp1"], ["exp2", "exp3"]], + [["exp1", "exp2"], ["exp3"]], + [["exp1", "exp2", "exp3"], []], + [[], []] # remove does nothing if no experiments are specified + ] +) +def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage, keep, expected_removed): # Setup: Run experiments - results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") - exp1_ref = first(exp_refs_by_rev(scm, first(results))) + refs = {} + for i in range(1, len(keep)+len(expected_removed)+1): + results = dvc.experiments.run(exp_stage.addressing, + params=[f"foo={i}"], name=f"exp{i}") + refs[f"exp{i}"] = first(exp_refs_by_rev(scm, first(results))) + assert scm.get_ref(str(refs[f"exp{i}"])) is not None - results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") - exp2_ref = first(exp_refs_by_rev(scm, first(results))) + removed = dvc.experiments.remove(exp_names=keep, keep=True) + assert sorted(removed) == sorted(expected_removed) - results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") - exp3_ref = first(exp_refs_by_rev(scm, first(results))) + for exp in expected_removed: + assert scm.get_ref(str(refs[exp])) is None - # Ensure experiments exist - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None - - # Keep "exp2" and remove others - removed = dvc.experiments.remove(exp_names=["exp2"], keep=True) - assert sorted(removed) == ["exp1", "exp3"] - - # Check remaining experiments - assert scm.get_ref(str(exp1_ref)) is None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is None - - -def test_keep_selected_multiple_by_name(tmp_dir, scm, dvc, exp_stage): - # Setup: Run experiments - results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") - exp1_ref = first(exp_refs_by_rev(scm, first(results))) - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") - exp2_ref = first(exp_refs_by_rev(scm, first(results))) - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") - exp3_ref = first(exp_refs_by_rev(scm, first(results))) - - # Ensure experiments exist - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None - - # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["exp1", "exp2"], keep=True) - assert removed == ["exp3"] - - # Check remaining experiments - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is None - - -def test_keep_selected_all_by_name(tmp_dir, scm, dvc, exp_stage): - # Setup: Run experiments - results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") - exp1_ref = first(exp_refs_by_rev(scm, first(results))) - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") - exp2_ref = first(exp_refs_by_rev(scm, first(results))) - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") - exp3_ref = first(exp_refs_by_rev(scm, first(results))) - - # Ensure experiments exist - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None - - # Keep "exp1" and "exp2" and remove "exp3" - removed = dvc.experiments.remove(exp_names=["exp1", "exp2", "exp3"], keep=True) - assert removed == [] - - # Check remaining experiments - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None + for exp in keep: + assert scm.get_ref(str(refs[exp])) is not None def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): - # Setup: Run experiments - results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") - exp1_ref = first(exp_refs_by_rev(scm, first(results))) - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") - exp2_ref = first(exp_refs_by_rev(scm, first(results))) - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") - exp3_ref = first(exp_refs_by_rev(scm, first(results))) - - # Ensure experiments exist - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None # non existent name should raise an error with pytest.raises(UnresolvedExpNamesError): dvc.experiments.remove(exp_names=["nonexistent"], keep=True) - # Check nothing has been deleted - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None - -def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage): +@pytest.mark.parametrize( + "num_exps, rev, num, expected_removed", + [ + [2, "exp1", 1, ["exp2"]], + [3, "exp3", 1, ["exp1", "exp2"]], + [3, "exp3", 2, ["exp1"]], + [3, "exp3", 3, []], + [3, "exp2", 2, ["exp3"]], + [4, "exp2", 2, ["exp3", "exp4"]], + [4, "exp4", 2, ["exp1", "exp2"]], + [1, None, 1, []] # remove does nothing if no experiments are specified + ] +) +def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage, num_exps, rev, + num, expected_removed): + + refs = {} + revs = {} # Setup: Run experiments and commit - results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") - exp1_ref = first(exp_refs_by_rev(scm, first(results))) - scm.commit("commit1") - - new_results = dvc.experiments.run( - exp_stage.addressing, params=["foo=2"], name="exp2" - ) - exp2_ref = first(exp_refs_by_rev(scm, first(new_results))) - new_rev = scm.get_rev() - - # Ensure experiments exist - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None + for i in range(1, num_exps + 1): + scm.commit(f"commit{i}") + results = dvc.experiments.run(exp_stage.addressing, + params=[f"foo={i}"], name=f"exp{i}") + refs[f"exp{i}"] = first(exp_refs_by_rev(scm, first(results))) + revs[f"exp{i}"] = scm.get_rev() + assert scm.get_ref(str(refs[f"exp{i}"])) is not None # Keep the experiment from the new revision - removed = dvc.experiments.remove(rev=new_rev, num=1, keep=True) - assert removed == ["exp1"] + removed = dvc.experiments.remove(rev=revs.get(rev), num=num, keep=True) + assert sorted(removed) == sorted(expected_removed) # Check remaining experiments - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp1_ref)) is None - - -def test_keep_selected_by_rev_multiple(tmp_dir, scm, dvc, exp_stage): - # Setup: Run experiments and commit - results = dvc.experiments.run(exp_stage.addressing, params=["foo=1"], name="exp1") - exp1_ref = first(exp_refs_by_rev(scm, first(results))) - scm.commit("commit1") - - results = dvc.experiments.run(exp_stage.addressing, params=["foo=2"], name="exp2") - exp2_ref = first(exp_refs_by_rev(scm, first(results))) - scm.commit("commit2") + for exp in expected_removed: + assert scm.get_ref(str(refs[exp])) is None - exp3_rev = scm.get_rev() - results = dvc.experiments.run(exp_stage.addressing, params=["foo=3"], name="exp3") - exp3_ref = first(exp_refs_by_rev(scm, first(results))) - scm.commit("commit3") - - # Ensure experiments exist - assert scm.get_ref(str(exp1_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp3_ref)) is not None - - # Keep the last 2, remove first - removed = dvc.experiments.remove(rev=exp3_rev, num=2, keep=True) - assert removed == ["exp1"] - - # Check remaining experiments - assert scm.get_ref(str(exp3_ref)) is not None - assert scm.get_ref(str(exp2_ref)) is not None - assert scm.get_ref(str(exp1_ref)) is None + for exp in refs.keys(): + if exp not in expected_removed: + assert scm.get_ref(str(refs[exp])) is not None From 61fda4629c80e6d72bd0657a222455036a00139d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 27 Nov 2024 23:01:58 +0000 Subject: [PATCH 21/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/repo/experiments/refs.py | 2 +- dvc/repo/experiments/remove.py | 2 +- tests/func/experiments/test_remove.py | 29 ++++++++++++++------------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/dvc/repo/experiments/refs.py b/dvc/repo/experiments/refs.py index ef7b281f62..3a34ff35a0 100644 --- a/dvc/repo/experiments/refs.py +++ b/dvc/repo/experiments/refs.py @@ -75,4 +75,4 @@ def __eq__(self, other): return self.baseline_sha == other.baseline_sha and self.name == other.name def __hash__(self): - return hash((self.baseline_sha, self.name)) \ No newline at end of file + return hash((self.baseline_sha, self.name)) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index dcdcdd398f..ecd7301ea9 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -73,7 +73,6 @@ def remove( # noqa: C901, PLR0912 exp_ref_list.extend(exp_refs(repo.scm, git_remote)) removed = [ref.name for ref in exp_ref_list] - if keep: exp_ref_list = list(set(exp_refs(repo.scm, git_remote)) - set(exp_ref_list)) removed = [ref.name for ref in exp_ref_list] @@ -94,6 +93,7 @@ def remove( # noqa: C901, PLR0912 return removed + def _resolve_exp_by_baseline( repo: "Repo", rev: list[str], diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index cc31324227..c185e93bb5 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -187,15 +187,16 @@ def test_remove_multi_rev(tmp_dir, scm, dvc, exp_stage): [["exp1"], ["exp2", "exp3"]], [["exp1", "exp2"], ["exp3"]], [["exp1", "exp2", "exp3"], []], - [[], []] # remove does nothing if no experiments are specified - ] + [[], []], # remove does nothing if no experiments are specified + ], ) def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage, keep, expected_removed): # Setup: Run experiments refs = {} - for i in range(1, len(keep)+len(expected_removed)+1): - results = dvc.experiments.run(exp_stage.addressing, - params=[f"foo={i}"], name=f"exp{i}") + for i in range(1, len(keep) + len(expected_removed) + 1): + results = dvc.experiments.run( + exp_stage.addressing, params=[f"foo={i}"], name=f"exp{i}" + ) refs[f"exp{i}"] = first(exp_refs_by_rev(scm, first(results))) assert scm.get_ref(str(refs[f"exp{i}"])) is not None @@ -210,7 +211,6 @@ def test_keep_selected_by_name(tmp_dir, scm, dvc, exp_stage, keep, expected_remo def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): - # non existent name should raise an error with pytest.raises(UnresolvedExpNamesError): dvc.experiments.remove(exp_names=["nonexistent"], keep=True) @@ -226,19 +226,20 @@ def test_keep_selected_by_nonexistent_name(tmp_dir, scm, dvc, exp_stage): [3, "exp2", 2, ["exp3"]], [4, "exp2", 2, ["exp3", "exp4"]], [4, "exp4", 2, ["exp1", "exp2"]], - [1, None, 1, []] # remove does nothing if no experiments are specified - ] + [1, None, 1, []], # remove does nothing if no experiments are specified + ], ) -def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage, num_exps, rev, - num, expected_removed): - +def test_keep_selected_by_rev( + tmp_dir, scm, dvc, exp_stage, num_exps, rev, num, expected_removed +): refs = {} revs = {} # Setup: Run experiments and commit for i in range(1, num_exps + 1): scm.commit(f"commit{i}") - results = dvc.experiments.run(exp_stage.addressing, - params=[f"foo={i}"], name=f"exp{i}") + results = dvc.experiments.run( + exp_stage.addressing, params=[f"foo={i}"], name=f"exp{i}" + ) refs[f"exp{i}"] = first(exp_refs_by_rev(scm, first(results))) revs[f"exp{i}"] = scm.get_rev() assert scm.get_ref(str(refs[f"exp{i}"])) is not None @@ -251,6 +252,6 @@ def test_keep_selected_by_rev(tmp_dir, scm, dvc, exp_stage, num_exps, rev, for exp in expected_removed: assert scm.get_ref(str(refs[exp])) is None - for exp in refs.keys(): + for exp in refs: if exp not in expected_removed: assert scm.get_ref(str(refs[exp])) is not None From c6c67b99b727ec55330639df88ba973cdb0fdcc5 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Thu, 28 Nov 2024 14:15:25 +0100 Subject: [PATCH 22/25] fixed linting issues --- tests/func/experiments/test_remove.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index c185e93bb5..683980a2d0 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -252,6 +252,6 @@ def test_keep_selected_by_rev( for exp in expected_removed: assert scm.get_ref(str(refs[exp])) is None - for exp in refs: + for exp, ref in refs.items(): if exp not in expected_removed: - assert scm.get_ref(str(refs[exp])) is not None + assert scm.get_ref(str(ref)) is not None From 9397e83c65d72bf75a380433b19114991ca6856e Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Fri, 29 Nov 2024 09:16:11 +0100 Subject: [PATCH 23/25] - --keep and --queue together raise an InvalidArgumentError - added a test to check if the error is raised - fixed CLI message --- dvc/commands/experiments/__init__.py | 3 +-- dvc/repo/experiments/remove.py | 6 +++++- tests/func/experiments/test_remove.py | 7 +++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index fc949e9e49..c604d8afe7 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -63,8 +63,7 @@ def add_keep_selection_flag(experiments_subcmd_parser): "--keep", action="store_true", default=False, - help="Keep the selected (committed, not queued) experiments " - "instead of removing them.", + help="Keep the selected experiments instead of removing them." ) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index ecd7301ea9..bacbe69424 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -6,7 +6,7 @@ from dvc.repo.scm_context import scm_context from dvc.scm import Git, iter_revs -from .exceptions import UnresolvedExpNamesError +from .exceptions import UnresolvedExpNamesError, InvalidArgumentError from .utils import exp_refs, exp_refs_by_baseline, push_refspec if TYPE_CHECKING: @@ -34,8 +34,12 @@ def remove( # noqa: C901, PLR0912 ) -> list[str]: removed: list[str] = [] + if all([keep, queue]): + raise InvalidArgumentError("Cannot use both `--keep` and `--queue`.") + if not any([exp_names, queue, all_commits, rev]): return removed + celery_queue: LocalCeleryQueue = repo.experiments.celery_queue if queue: diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 683980a2d0..e39dbf1572 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -255,3 +255,10 @@ def test_keep_selected_by_rev( for exp, ref in refs.items(): if exp not in expected_removed: assert scm.get_ref(str(ref)) is not None + +def test_remove_with_queue_and_keep(tmp_dir, scm, dvc, exp_stage): + # This should raise an exception, until decided otherwise + + with pytest.raises(InvalidArgumentError): + dvc.experiments.remove(queue=True, keep=True) + From 9365b8a72a40d3d18b4880267f8fccd9bed8f400 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 29 Nov 2024 08:46:14 +0000 Subject: [PATCH 24/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/commands/experiments/__init__.py | 2 +- dvc/repo/experiments/remove.py | 2 +- tests/func/experiments/test_remove.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/commands/experiments/__init__.py b/dvc/commands/experiments/__init__.py index c604d8afe7..0413765d2b 100644 --- a/dvc/commands/experiments/__init__.py +++ b/dvc/commands/experiments/__init__.py @@ -63,7 +63,7 @@ def add_keep_selection_flag(experiments_subcmd_parser): "--keep", action="store_true", default=False, - help="Keep the selected experiments instead of removing them." + help="Keep the selected experiments instead of removing them.", ) diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index bacbe69424..cd8ca07e8b 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -6,7 +6,7 @@ from dvc.repo.scm_context import scm_context from dvc.scm import Git, iter_revs -from .exceptions import UnresolvedExpNamesError, InvalidArgumentError +from .exceptions import InvalidArgumentError, UnresolvedExpNamesError from .utils import exp_refs, exp_refs_by_baseline, push_refspec if TYPE_CHECKING: diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index e39dbf1572..076ac1ecb3 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -256,9 +256,9 @@ def test_keep_selected_by_rev( if exp not in expected_removed: assert scm.get_ref(str(ref)) is not None + def test_remove_with_queue_and_keep(tmp_dir, scm, dvc, exp_stage): # This should raise an exception, until decided otherwise with pytest.raises(InvalidArgumentError): dvc.experiments.remove(queue=True, keep=True) - From fe5c5d743a01133be4ab6bcd5311f34fc29fdef8 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Fri, 29 Nov 2024 10:13:16 +0100 Subject: [PATCH 25/25] re-run gh tests. Some tests which did not involve my changes started failing while they were passing fine before. --- tests/func/experiments/test_remove.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/func/experiments/test_remove.py b/tests/func/experiments/test_remove.py index 076ac1ecb3..1864cc541d 100644 --- a/tests/func/experiments/test_remove.py +++ b/tests/func/experiments/test_remove.py @@ -259,6 +259,5 @@ def test_keep_selected_by_rev( def test_remove_with_queue_and_keep(tmp_dir, scm, dvc, exp_stage): # This should raise an exception, until decided otherwise - with pytest.raises(InvalidArgumentError): dvc.experiments.remove(queue=True, keep=True)