run: make --deterministic default

Signed-off-by: Ruslan Kuprieiev <[email protected]>

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,

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

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

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
 

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()