From 298ae356cd17f57e20d0f52140210b73bd8d3c3e Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Mon, 3 Dec 2018 04:50:48 +0200 Subject: [PATCH] run: make --deterministic default Signed-off-by: Ruslan Kuprieiev --- dvc/cli.py | 6 ---- dvc/command/run.py | 3 +- dvc/project.py | 6 ++-- dvc/stage.py | 66 ++++++++++++++++++++------------------- tests/test_run.py | 77 +++++++++++++++++++--------------------------- 5 files changed, 70 insertions(+), 88 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index 214dc04ca..4f3ceb90b 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -271,12 +271,6 @@ def parse_args(argv=None): help="Automatic 'yes' answer to all prompts. E.g. " "when '.dvc' file exists and dvc asks if you " "want to overwrite it.") - run_parser.add_argument( - '--deterministic', - action='store_true', - default=False, - help="Use cached result if previously run with " - "current dependencies and command.") run_parser.add_argument( 'command', nargs=argparse.REMAINDER, diff --git a/dvc/command/run.py b/dvc/command/run.py index 2a052c4ab..0fa2c2036 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -31,8 +31,7 @@ class CmdRun(CmdBase): deps=self.args.deps, fname=self.args.file, cwd=self.args.cwd, - no_exec=self.args.no_exec, - deterministic=self.args.deterministic) + no_exec=self.args.no_exec) except DvcException as ex: self.project.logger.error('Failed to run command', ex) return 1 diff --git a/dvc/project.py b/dvc/project.py index 01891ec3e..8dbc8d429 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -363,8 +363,7 @@ class Project(object): fname=None, cwd=os.curdir, no_exec=False, - overwrite=False, - deterministic=False): + overwrite=False): from dvc.stage import Stage with self.state: @@ -376,8 +375,7 @@ class Project(object): outs_no_cache=outs_no_cache, metrics_no_cache=metrics_no_cache, deps=deps, - overwrite=overwrite, - deterministic=deterministic) + overwrite=overwrite) if stage is None: return None diff --git a/dvc/stage.py b/dvc/stage.py index b58d1b636..09d3b846c 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -29,6 +29,12 @@ class StageFileDoesNotExistError(DvcException): super(StageFileDoesNotExistError, self).__init__(msg) +class StageFileAlreadyExistsError(DvcException): + def __init__(self, relpath): + msg = "Stage '{}' already exists".format(relpath) + super(StageFileAlreadyExistsError, self).__init__(msg) + + class StageFileIsNotDvcFileError(DvcException): def __init__(self, fname): msg = "'{}' is not a dvc file".format(fname) @@ -149,7 +155,10 @@ class Stage(object): len(self.deps) == 1 and \ len(self.outs) == 1 - def _changed_deps(self, print_info, log): + def _changed_deps(self, log): + if self.locked: + return False + if self.is_callback: msg = "Dvc file '{}' is a 'callback' stage (has a command and " \ "no dependencies) and thus always considered as changed." @@ -159,37 +168,35 @@ class Stage(object): for dep in self.deps: if not dep.changed(): continue - if print_info: - msg = "Dependency '{}' of '{}' changed." - log(msg.format(dep, self.relpath)) + log("Dependency '{}' of '{}' changed.".format(dep, self.relpath)) return True return False - def changed(self, print_info=False): - ret = False + def _changed_outs(self, log): + for out in self.outs: + if not out.changed(): + continue + log("Output '{}' of '{}' changed.".format(out, self.relpath)) + return True + return False + + def _changed_md5(self, log): + if self.changed_md5(): + log("Dvc file '{}' changed.".format(self.relpath)) + return True + return False + + def changed(self, print_info=False): if print_info: log = self.project.logger.info else: log = self.project.logger.debug - if not self.locked: - ret = self._changed_deps(print_info, log) - - for out in self.outs: - if not out.changed(): - continue - if print_info: - msg = "Output '{}' of '{}' changed." - log(msg.format(out, self.relpath)) - ret = True - - if self.changed_md5(): - if print_info: - msg = "Dvc file '{}' changed." - log(msg.format(self.relpath)) - ret = True + ret = any([self._changed_deps(log), + self._changed_outs(log), + self._changed_md5(log)]) if ret: msg = "Stage '{}' changed.".format(self.relpath) @@ -299,7 +306,7 @@ class Stage(object): from dvc.remote.s3 import RemoteS3 old = Stage.load(self.project, self.path) - if old.changed(): + if old._changed_outs(log=self.project.logger.debug): return False # NOTE: need to save checksums for deps in order to compare them @@ -332,8 +339,7 @@ class Stage(object): cwd=os.curdir, locked=False, add=False, - overwrite=True, - deterministic=False): + overwrite=True): stage = Stage(project=project, cwd=cwd, @@ -361,17 +367,15 @@ class Stage(object): stage.cwd = cwd stage.path = path - if os.path.exists(path): - if deterministic and stage.is_cached(): + if os.path.exists(path) and not overwrite: + if stage.is_cached(): Logger.info('Stage is cached, skipping.') return None - relpath = os.path.relpath(path) msg = "'{}' already exists. " \ "Do you wish to run the command and overwrite it?" - if not overwrite \ - and not project.prompt.prompt(msg.format(relpath), False): - raise DvcException("'{}' already exists".format(relpath)) + if not project.prompt.prompt(msg.format(stage.relpath), False): + raise StageFileAlreadyExistsError(stage.relpath) return stage diff --git a/tests/test_run.py b/tests/test_run.py index b47f76703..5f7ee848f 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -9,7 +9,7 @@ from dvc.project import Project from dvc.main import main from dvc.utils import file_md5 from dvc.stage import Stage, StageFileBadNameError, MissingDep -from dvc.stage import StageBadCwdError +from dvc.stage import StageBadCwdError, StageFileAlreadyExistsError from dvc.command.run import CmdRun from dvc.exceptions import (OutputDuplicationError, CircularDependencyError, @@ -184,20 +184,6 @@ class TestCmdRun(TestDvc): ret = main(['run', 'mycmd']) self.assertEqual(ret, 252) - @mock.patch.object(Project, 'run') - def test_deterministic(self, mock_run): - ret = main(['run', - '-d', self.FOO, - '-d', self.CODE, - '--deterministic', - '-o', 'out', - '-f', 'out.dvc', - 'python', self.CODE, self.FOO, 'out']) - self.assertEqual(ret, 0) - mock_run.assert_called_once() - args, kwargs = mock_run.call_args - self.assertIn(('deterministic', True), kwargs.items()) - class TestRunDeterministicBase(TestDvc): def setUp(self): @@ -205,77 +191,78 @@ class TestRunDeterministicBase(TestDvc): self.out_file = 'out' self.stage_file = self.out_file + '.dvc' self.cmd = 'python {} {} {}'.format(self.CODE, self.FOO, self.out_file) - self.deterministic = True self.deps = [self.FOO, self.CODE] self.outs = [self.out_file] + self.overwrite = False - self._run(check=False) - self.assertTrue(self.stage is not None) - self.stage_file_mtime = os.path.getmtime(self.stage_file) - self.out_file_mtime = os.path.getmtime(self.out_file) + self._run() - def _run(self, should_run=True, check=True): + def _run(self): self.stage = self.dvc.run(cmd=self.cmd, fname=self.stage_file, - deterministic=self.deterministic, - overwrite=True, + overwrite=self.overwrite, deps=self.deps, outs=self.outs) - if not check: - return - - stage_file_mtime = os.path.getmtime(self.stage_file) - out_file_mtime = os.path.getmtime(self.out_file) - - if should_run: - self.assertTrue(self.stage is not None) - self.assertNotEqual(self.stage_file_mtime, stage_file_mtime) - self.assertNotEqual(self.out_file_mtime, out_file_mtime) - else: - self.assertTrue(self.stage is None) - self.assertEqual(self.stage_file_mtime, stage_file_mtime) - self.assertEqual(self.out_file_mtime, out_file_mtime) - class TestRunDeterministic(TestRunDeterministicBase): def test(self): - self._run(False) + self._run() + + +class TestRunDeterministicOverwrite(TestRunDeterministicBase): + def test(self): + self.overwrite = True + self._run() + + +class TestRunDeterministicCallback(TestRunDeterministicBase): + def test(self): + self.stage.remove() + self.deps = [] + self._run() + self._run() class TestRunDeterministicChangedDep(TestRunDeterministicBase): def test(self): os.unlink(self.FOO) shutil.copy(self.BAR, self.FOO) - self._run() + with self.assertRaises(StageFileAlreadyExistsError): + self._run() class TestRunDeterministicChangedDepsList(TestRunDeterministicBase): def test(self): self.deps = [self.BAR, self.CODE] - self._run() + with self.assertRaises(StageFileAlreadyExistsError): + self._run() class TestRunDeterministicNewDep(TestRunDeterministicBase): def test(self): self.deps = [self.FOO, self.BAR, self.CODE] - self._run() + with self.assertRaises(StageFileAlreadyExistsError): + self._run() class TestRunDeterministicRemoveDep(TestRunDeterministicBase): def test(self): self.deps = [self.CODE] - self._run() + with self.assertRaises(StageFileAlreadyExistsError): + self._run() class TestRunDeterministicChangedOut(TestRunDeterministicBase): def test(self): os.unlink(self.out_file) self.out_file_mtime = None - self._run() + with self.assertRaises(StageFileAlreadyExistsError): + self._run() class TestRunDeterministicChangedCmd(TestRunDeterministicBase): def test(self): self.cmd += ' arg' - self._run() + with self.assertRaises(StageFileAlreadyExistsError): + self._run()