From 8252ae5f3cb4fa22d1d13fbdc5d0a2a9601fe190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Rowlands=20=28=EB=B3=80=EA=B8=B0=ED=98=B8=29?= Date: Thu, 3 Dec 2020 17:21:19 +0900 Subject: [PATCH] experiments: enable by default, support no_scm (#5019) * experiments: enable by default, support no_scm * mark experiments option obsolete --- dvc/command/experiments.py | 17 ++---------- dvc/config.py | 2 +- dvc/repo/__init__.py | 14 +++++----- dvc/repo/experiments/__init__.py | 31 +++++----------------- dvc/repo/plots/diff.py | 8 +----- tests/func/experiments/test_experiments.py | 25 +++++++++++++++++ tests/unit/command/test_experiments.py | 12 ++++----- 7 files changed, 48 insertions(+), 61 deletions(-) diff --git a/dvc/command/experiments.py b/dvc/command/experiments.py index c3d2b5747..b89b95c17 100644 --- a/dvc/command/experiments.py +++ b/dvc/command/experiments.py @@ -300,9 +300,6 @@ class CmdExperimentsShow(CmdBase): from dvc.utils.pager import pager - if not self.repo.experiments: - return 0 - try: all_experiments = self.repo.experiments.show( all_branches=self.args.all_branches, @@ -355,8 +352,6 @@ class CmdExperimentsShow(CmdBase): class CmdExperimentsApply(CmdBase): def run(self): - if not self.repo.experiments: - return 0 self.repo.experiments.apply(self.args.experiment) @@ -402,8 +397,6 @@ def _show_diff( class CmdExperimentsDiff(CmdBase): def run(self): - if not self.repo.experiments: - return 0 try: diff = self.repo.experiments.diff( @@ -445,8 +438,6 @@ class CmdExperimentsDiff(CmdBase): class CmdExperimentsRun(CmdRepro): def run(self): - if not self.repo.experiments: - return 0 saved_dir = os.path.realpath(os.curdir) os.chdir(self.args.cwd) @@ -483,9 +474,6 @@ class CmdExperimentsGC(CmdRepro): def run(self): from dvc.repo.gc import _raise_error_if_all_disabled - if not self.repo.experiments: - return 0 - _raise_error_if_all_disabled( all_branches=self.args.all_branches, all_tags=self.args.all_tags, @@ -536,8 +524,6 @@ class CmdExperimentsGC(CmdRepro): class CmdExperimentsBranch(CmdBase): def run(self): - if not self.repo.experiments: - return 0 self.repo.experiments.branch(self.args.experiment, self.args.branch) @@ -545,7 +531,7 @@ class CmdExperimentsBranch(CmdBase): def add_parser(subparsers, parent_parser): - EXPERIMENTS_HELP = "Commands to display and compare experiments." + EXPERIMENTS_HELP = "Commands to run and compare experiments." experiments_parser = subparsers.add_parser( "experiments", @@ -553,6 +539,7 @@ def add_parser(subparsers, parent_parser): aliases=["exp"], description=append_doc_link(EXPERIMENTS_HELP, "experiments"), formatter_class=argparse.RawDescriptionHelpFormatter, + help=EXPERIMENTS_HELP, ) experiments_subparsers = experiments_parser.add_subparsers( diff --git a/dvc/config.py b/dvc/config.py index 799924ab5..f7e7f60cd 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -141,7 +141,7 @@ SCHEMA = { Optional("hardlink_lock", default=False): Bool, Optional("no_scm", default=False): Bool, Optional("autostage", default=False): Bool, - Optional("experiments", default=False): Bool, + Optional("experiments"): Bool, # obsoleted Optional("check_update", default=True): Bool, }, "cache": { diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 78dd4b7aa..41daa23a7 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -133,7 +133,6 @@ class Repo: from dvc.cache import Cache from dvc.data_cloud import DataCloud from dvc.lock import LockNoop, make_lock - from dvc.repo.experiments import Experiments from dvc.repo.metrics import Metrics from dvc.repo.params import Params from dvc.repo.plots import Plots @@ -179,11 +178,6 @@ class Repo: self.state = State(self) self.stage_cache = StageCache(self) - try: - self.experiments = Experiments(self) - except NotImplementedError: - self.experiments = None - self._ignore() self.metrics = Metrics(self) @@ -198,6 +192,12 @@ class Repo: no_scm = self.config["core"].get("no_scm", False) return self._scm if self._scm else SCM(self.root_dir, no_scm=no_scm) + @cached_property + def experiments(self): + from dvc.repo.experiments import Experiments + + return Experiments(self) + @property def tree(self) -> "BaseTree": return self._tree @@ -258,8 +258,6 @@ class Repo: self.config.files["local"], self.tmp_dir, ] - if self.experiments: - flist.append(self.experiments.exp_dir) if path_isin(self.cache.local.cache_dir, self.root_dir): flist += [self.cache.local.cache_dir] diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py index e32357834..f44ac1904 100644 --- a/dvc/repo/experiments/__init__.py +++ b/dvc/repo/experiments/__init__.py @@ -28,7 +28,7 @@ from dvc.repo.experiments.base import ( ) from dvc.repo.experiments.executor import BaseExecutor, LocalExecutor from dvc.stage.run import CheckpointKilledError -from dvc.utils import env2bool, relpath +from dvc.utils import relpath logger = logging.getLogger(__name__) @@ -72,7 +72,6 @@ class Experiments: repo (dvc.repo.Repo): repo instance that these experiments belong to. """ - EXPERIMENTS_DIR = "experiments" STASH_EXPERIMENT_FORMAT = "dvc-exp:{rev}:{baseline_rev}:{name}" STASH_EXPERIMENT_RE = re.compile( r"(?:commit: )" @@ -92,12 +91,10 @@ class Experiments: def __init__(self, repo): from dvc.lock import make_lock + from dvc.scm.base import NoSCMError - if not ( - env2bool("DVC_TEST") - or repo.config["core"].get("experiments", False) - ): - raise NotImplementedError + if repo.config["core"].get("no_scm", False): + raise NoSCMError self.repo = repo self.scm_lock = make_lock( @@ -105,10 +102,6 @@ class Experiments: tmp_dir=self.repo.tmp_dir, ) - @cached_property - def exp_dir(self): - return os.path.join(self.repo.dvc_dir, self.EXPERIMENTS_DIR) - @property def scm(self): return self.repo.scm @@ -117,23 +110,13 @@ class Experiments: def dvc_dir(self): return relpath(self.repo.dvc_dir, self.repo.scm.root_dir) - @cached_property - def exp_dvc_dir(self): - return os.path.join(self.exp_dir, self.dvc_dir) - - @property - def exp_dvc(self): - return self.repo - @contextmanager def chdir(self): yield @cached_property def args_file(self): - return os.path.join( - self.exp_dvc.tmp_dir, BaseExecutor.PACKED_ARGS_FILE - ) + return os.path.join(self.repo.tmp_dir, BaseExecutor.PACKED_ARGS_FILE) @cached_property def stash(self): @@ -256,10 +239,10 @@ class Experiments: logger.debug("Using experiment params '%s'", params) for params_fname in params: - path = PathInfo(self.exp_dvc.root_dir) / params_fname + path = PathInfo(self.repo.root_dir) / params_fname suffix = path.suffix.lower() modify_data = MODIFIERS[suffix] - with modify_data(path, tree=self.exp_dvc.tree) as data: + with modify_data(path, tree=self.repo.tree) as data: benedict(data).merge(params[params_fname], overwrite=True) # Force params file changes to be staged in git diff --git a/dvc/repo/plots/diff.py b/dvc/repo/plots/diff.py index b08cfa6ec..73dfabcfb 100644 --- a/dvc/repo/plots/diff.py +++ b/dvc/repo/plots/diff.py @@ -12,12 +12,6 @@ def _revisions(repo, revs, experiment): def diff(repo, *args, revs=None, experiment=False, **kwargs): - if experiment: - # use experiments repo brancher with templates from the main repo - kwargs["templates"] = repo.plots.templates - plots_repo = repo.experiments.exp_dvc - else: - plots_repo = repo - return plots_repo.plots.show( + return repo.plots.show( *args, revs=_revisions(repo, revs, experiment), **kwargs ) diff --git a/tests/func/experiments/test_experiments.py b/tests/func/experiments/test_experiments.py index 032e7e82e..6e6e318d8 100644 --- a/tests/func/experiments/test_experiments.py +++ b/tests/func/experiments/test_experiments.py @@ -305,3 +305,28 @@ def test_branch(tmp_dir, scm, dvc, exp_stage): assert "branch-ref-b" in scm.list_branches() assert scm.resolve_rev("branch-ref-b") == exp_b + + +def test_no_scm(tmp_dir): + from dvc.repo import Repo as DvcRepo + from dvc.scm.base import NoSCMError + + dvc = DvcRepo.init(no_scm=True) + + with pytest.raises(NoSCMError): + dvc.experiments.run() + + with pytest.raises(NoSCMError): + dvc.experiments.apply() + + with pytest.raises(NoSCMError): + dvc.experiments.branch() + + with pytest.raises(NoSCMError): + dvc.experiments.diff() + + with pytest.raises(NoSCMError): + dvc.experiments.show() + + with pytest.raises(NoSCMError): + dvc.experiments.gc() diff --git a/tests/unit/command/test_experiments.py b/tests/unit/command/test_experiments.py index 0f3408830..a2d00e2bc 100644 --- a/tests/unit/command/test_experiments.py +++ b/tests/unit/command/test_experiments.py @@ -15,7 +15,7 @@ from dvc.exceptions import InvalidArgumentError from .test_repro import default_arguments as repro_arguments -def test_experiments_apply(dvc, mocker): +def test_experiments_apply(dvc, scm, mocker): cli_args = parse_args(["experiments", "apply", "exp_rev"]) assert cli_args.func == CmdExperimentsApply @@ -27,7 +27,7 @@ def test_experiments_apply(dvc, mocker): m.assert_called_once_with(cmd.repo, "exp_rev") -def test_experiments_diff(dvc, mocker): +def test_experiments_diff(dvc, scm, mocker): cli_args = parse_args( [ "experiments", @@ -54,7 +54,7 @@ def test_experiments_diff(dvc, mocker): ) -def test_experiments_show(dvc, mocker): +def test_experiments_show(dvc, scm, mocker): cli_args = parse_args( [ "experiments", @@ -84,7 +84,7 @@ def test_experiments_show(dvc, mocker): @pytest.mark.parametrize( "args, resume", [(["exp", "run"], None), (["exp", "resume"], ":last")] ) -def test_experiments_run(dvc, mocker, args, resume): +def test_experiments_run(dvc, scm, mocker, args, resume): default_arguments = { "params": [], "name": None, @@ -105,7 +105,7 @@ def test_experiments_run(dvc, mocker, args, resume): ) -def test_experiments_gc(dvc, mocker): +def test_experiments_gc(dvc, scm, mocker): cli_args = parse_args( [ "exp", @@ -140,7 +140,7 @@ def test_experiments_gc(dvc, mocker): cmd.run() -def test_experiments_branch(dvc, mocker): +def test_experiments_branch(dvc, scm, mocker): cli_args = parse_args(["experiments", "branch", "expname", "branchname"]) assert cli_args.func == CmdExperimentsBranch