From ed8052c2292d32f0ff02a3e2ab67d740be4727bd Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 4 Aug 2018 23:29:01 +0300 Subject: [PATCH 1/3] repro: introduce --dry option Fixes #972 Related to #894 Signed-off-by: Ruslan Kuprieiev --- dvc/cli.py | 6 ++++++ dvc/command/repro.py | 3 ++- dvc/project.py | 19 +++++++++++-------- dvc/stage.py | 37 +++++++++++++++++++++---------------- tests/test_repro.py | 13 +++++++++++++ 5 files changed, 53 insertions(+), 25 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index 26574d0c7..48598f664 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -324,6 +324,12 @@ def parse_args(argv=None): action='store_true', default=False, help='Show metrics after reproduction') + repro_parser.add_argument( + '--dry', + action='store_true', + default=False, + help='Only print the commands that would be executed ' + 'without actually executing.') repro_parser.set_defaults(func=CmdRepro) # Remove diff --git a/dvc/command/repro.py b/dvc/command/repro.py index aed5390f3..1844e4ddd 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -17,7 +17,8 @@ class CmdRepro(CmdBase): try: self.project.reproduce(target, recursive=recursive, - force=self.args.force) + force=self.args.force, + dry=self.args.dry) if self.args.metrics: self.project.metrics_show() except ReproductionError as ex: diff --git a/dvc/project.py b/dvc/project.py index 6ad5b3e50..4f07eb926 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -235,7 +235,7 @@ class Project(object): stage.dump() return stage - def _reproduce_stage(self, stages, node, force): + def _reproduce_stage(self, stages, node, force, dry): stage = stages[node] if stage.locked: @@ -243,13 +243,16 @@ class Project(object): 'going to be reproduced.' self.logger.warn(msg.format(stage.relpath)) - stage = stage.reproduce(force=force) + stage = stage.reproduce(force=force, dry=dry) if not stage: return [] - stage.dump() + + if not dry: + stage.dump() + return [stage] - def reproduce(self, target, recursive=True, force=False): + def reproduce(self, target, recursive=True, force=False, dry=False): import networkx as nx G = self.graph()[1] @@ -259,17 +262,17 @@ class Project(object): raise NotDvcFileError(target) if recursive: - return self._reproduce_stages(G, stages, node, force) + return self._reproduce_stages(G, stages, node, force, dry) - return self._reproduce_stage(stages, node, force) + return self._reproduce_stage(stages, node, force, dry) - def _reproduce_stages(self, G, stages, node, force): + def _reproduce_stages(self, G, stages, node, force, dry): import networkx as nx result = [] for n in nx.dfs_postorder_nodes(G, node): try: - result += self._reproduce_stage(stages, n, force) + result += self._reproduce_stage(stages, n, force, dry) except Exception as ex: raise ReproductionError(stages[n].relpath, ex) return result diff --git a/dvc/stage.py b/dvc/stage.py index 5866e45d8..f3b19816e 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -150,17 +150,17 @@ class Stage(object): self.remove_outs(ignore_remove=True) os.unlink(self.path) - def reproduce(self, force=False): + def reproduce(self, force=False, dry=False): if not self.changed() and not force: return None - if (self.cmd or self.is_import) and not self.locked: + if (self.cmd or self.is_import) and not self.locked and not dry: # Removing outputs only if we actually have command to reproduce self.remove_outs(ignore_remove=False) self.project.logger.info(u'Reproducing \'{}\''.format(self.relpath)) - self.run() + self.run(dry=dry) msg = u'\'{}\' was reproduced'.format(self.relpath) self.project.logger.debug(msg) @@ -297,35 +297,40 @@ class Stage(object): for out in self.outs: out.save() - def run(self): + def run(self, dry=False): if self.locked: msg = u'Verifying outputs in locked stage \'{}\'' self.project.logger.info(msg.format(self.relpath)) - self.check_missing_outputs() + if not dry: + self.check_missing_outputs() elif self.is_import: msg = u'Importing \'{}\' -> \'{}\'' self.project.logger.info(msg.format(self.deps[0].path, self.outs[0].path)) - self.deps[0].download(self.outs[0].path_info) + if not dry: + self.deps[0].download(self.outs[0].path_info) elif self.is_data_source: msg = u'Verifying data sources in \'{}\''.format(self.relpath) self.project.logger.info(msg) - self.check_missing_outputs() + if not dry: + self.check_missing_outputs() else: msg = u'Running command:\n\t{}'.format(self.cmd) self.project.logger.info(msg) - p = subprocess.Popen(self.cmd, - cwd=self.cwd, - shell=True, - env=os.environ, - executable=os.getenv('SHELL')) - p.communicate() - if p.returncode != 0: - raise StageCmdFailedError(self) + if not dry: + p = subprocess.Popen(self.cmd, + cwd=self.cwd, + shell=True, + env=os.environ, + executable=os.getenv('SHELL')) + p.communicate() + if p.returncode != 0: + raise StageCmdFailedError(self) - self.save() + if not dry: + self.save() def check_missing_outputs(self): outs = [out for out in self.outs if not out.exists] diff --git a/tests/test_repro.py b/tests/test_repro.py index a3f2fc3e8..9baced2fc 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -125,6 +125,19 @@ class TestReproChangedData(TestRepro): shutil.copyfile(self.BAR, self.FOO) +class TestReproDry(TestReproChangedData): + def test(self): + self.swap_foo_with_bar() + + stages = self.dvc.reproduce(self.file1_stage, dry=True) + + self.assertTrue(len(stages), 2) + self.assertFalse(filecmp.cmp(self.file1, self.BAR)) + + ret = main(['repro', '--dry']) + self.assertFalse(filecmp.cmp(self.file1, self.BAR)) + + class TestReproChangedDeepData(TestReproChangedData): def test(self): file2 = 'file2' From aab04a22adb107026e85d16993def32b2360a01d Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 4 Aug 2018 23:55:21 +0300 Subject: [PATCH 2/3] repro: introduce --prompt Fixes #894 Signed-off-by: Ruslan Kuprieiev --- dvc/cli.py | 7 +++++++ dvc/command/repro.py | 3 ++- dvc/project.py | 28 +++++++++++++++++++++------- dvc/stage.py | 7 ++++++- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index 48598f664..32e07712b 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -330,6 +330,13 @@ def parse_args(argv=None): default=False, help='Only print the commands that would be executed ' 'without actually executing.') + repro_parser.add_argument( + '-i', + '--interactive', + action='store_true', + default=False, + help='Ask for confirmation before reproducing each ' + 'stage.') repro_parser.set_defaults(func=CmdRepro) # Remove diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 1844e4ddd..c9ef91839 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -18,7 +18,8 @@ class CmdRepro(CmdBase): self.project.reproduce(target, recursive=recursive, force=self.args.force, - dry=self.args.dry) + dry=self.args.dry, + interactive=self.args.interactive) if self.args.metrics: self.project.metrics_show() except ReproductionError as ex: diff --git a/dvc/project.py b/dvc/project.py index 4f07eb926..b2976f31c 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -235,7 +235,7 @@ class Project(object): stage.dump() return stage - def _reproduce_stage(self, stages, node, force, dry): + def _reproduce_stage(self, stages, node, force, dry, interactive): stage = stages[node] if stage.locked: @@ -243,7 +243,7 @@ class Project(object): 'going to be reproduced.' self.logger.warn(msg.format(stage.relpath)) - stage = stage.reproduce(force=force, dry=dry) + stage = stage.reproduce(force=force, dry=dry, interactive=interactive) if not stage: return [] @@ -252,7 +252,12 @@ class Project(object): return [stage] - def reproduce(self, target, recursive=True, force=False, dry=False): + def reproduce(self, + target, + recursive=True, + force=False, + dry=False, + interactive=False): import networkx as nx G = self.graph()[1] @@ -262,17 +267,26 @@ class Project(object): raise NotDvcFileError(target) if recursive: - return self._reproduce_stages(G, stages, node, force, dry) + return self._reproduce_stages(G, + stages, + node, + force, + dry, + interactive) - return self._reproduce_stage(stages, node, force, dry) + return self._reproduce_stage(stages, node, force, dry, interactive) - def _reproduce_stages(self, G, stages, node, force, dry): + def _reproduce_stages(self, G, stages, node, force, dry, interactive): import networkx as nx result = [] for n in nx.dfs_postorder_nodes(G, node): try: - result += self._reproduce_stage(stages, n, force, dry) + result += self._reproduce_stage(stages, + n, + force, + dry, + interactive) except Exception as ex: raise ReproductionError(stages[n].relpath, ex) return result diff --git a/dvc/stage.py b/dvc/stage.py index f3b19816e..41de32cbe 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -150,7 +150,7 @@ class Stage(object): self.remove_outs(ignore_remove=True) os.unlink(self.path) - def reproduce(self, force=False, dry=False): + def reproduce(self, force=False, dry=False, interactive=False): if not self.changed() and not force: return None @@ -158,6 +158,11 @@ class Stage(object): # Removing outputs only if we actually have command to reproduce self.remove_outs(ignore_remove=False) + msg = "Going to reproduce '{}'. Are you sure you want to continue?" + msg = msg.format(self.relpath) + if interactive and not prompt(msg): + raise DvcException('Reproduction aborted by the user') + self.project.logger.info(u'Reproducing \'{}\''.format(self.relpath)) self.run(dry=dry) From f5bb3e45015419bc8ddb384f3152673404d330ab Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sun, 5 Aug 2018 00:52:29 +0300 Subject: [PATCH 3/3] tests: use shallow=False with filecmp.cmp() Signed-off-by: Ruslan Kuprieiev --- tests/test_add.py | 1 - tests/test_checkout.py | 2 +- tests/test_import.py | 1 - tests/test_repro.py | 32 ++++++++++++++++---------------- tests/test_run.py | 4 ++-- tests/test_utils.py | 4 ++-- 6 files changed, 21 insertions(+), 23 deletions(-) diff --git a/tests/test_add.py b/tests/test_add.py index 1b1d72fdc..6b2be9991 100644 --- a/tests/test_add.py +++ b/tests/test_add.py @@ -1,7 +1,6 @@ import os import stat import shutil -import filecmp from dvc.main import main from dvc.utils import file_md5 diff --git a/tests/test_checkout.py b/tests/test_checkout.py index c24c0fcb4..fc226ae39 100644 --- a/tests/test_checkout.py +++ b/tests/test_checkout.py @@ -32,7 +32,7 @@ class TestCheckout(TestRepro): def _test_checkout(self): self.assertTrue(os.path.isfile(self.FOO)) - self.assertTrue(filecmp.cmp(self.FOO, self.orig)) + self.assertTrue(filecmp.cmp(self.FOO, self.orig, shallow=False)) class TestCheckoutSingleStage(TestCheckout): diff --git a/tests/test_import.py b/tests/test_import.py index fc75f536e..18a4581b5 100644 --- a/tests/test_import.py +++ b/tests/test_import.py @@ -1,7 +1,6 @@ import os import stat import shutil -import filecmp from dvc.main import main from dvc.command.imp import CmdImport diff --git a/tests/test_repro.py b/tests/test_repro.py index 9baced2fc..76ed53c6d 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -61,7 +61,7 @@ class TestReproDepUnderDir(TestDvc): deps=[self.DATA, self.CODE], cmd='python {} {} {}'.format(self.CODE, self.DATA, self.file1)) - self.assertTrue(filecmp.cmp(self.file1, self.DATA)) + self.assertTrue(filecmp.cmp(self.file1, self.DATA, shallow=False)) os.unlink(self.DATA) shutil.copyfile(self.FOO, self.DATA) @@ -70,7 +70,7 @@ class TestReproDepUnderDir(TestDvc): stages = self.dvc.reproduce(self.file1_stage) self.assertEqual(len(stages), 2) - self.assertTrue(filecmp.cmp(self.file1, self.FOO)) + self.assertTrue(filecmp.cmp(self.file1, self.FOO, shallow=False)) class TestReproNoDeps(TestRepro): @@ -101,7 +101,7 @@ class TestReproChangedCode(TestRepro): stages = self.dvc.reproduce(self.file1_stage) - self.assertTrue(filecmp.cmp(self.file1, self.BAR)) + self.assertTrue(filecmp.cmp(self.file1, self.BAR, shallow=False)) self.assertEqual(len(stages), 1) def swap_code(self): @@ -117,7 +117,7 @@ class TestReproChangedData(TestRepro): stages = self.dvc.reproduce(self.file1_stage) - self.assertTrue(filecmp.cmp(self.file1, self.BAR)) + self.assertTrue(filecmp.cmp(self.file1, self.BAR, shallow=False)) self.assertEqual(len(stages), 2) def swap_foo_with_bar(self): @@ -132,10 +132,10 @@ class TestReproDry(TestReproChangedData): stages = self.dvc.reproduce(self.file1_stage, dry=True) self.assertTrue(len(stages), 2) - self.assertFalse(filecmp.cmp(self.file1, self.BAR)) + self.assertFalse(filecmp.cmp(self.file1, self.BAR, shallow=False)) ret = main(['repro', '--dry']) - self.assertFalse(filecmp.cmp(self.file1, self.BAR)) + self.assertFalse(filecmp.cmp(self.file1, self.BAR, shallow=False)) class TestReproChangedDeepData(TestReproChangedData): @@ -151,8 +151,8 @@ class TestReproChangedDeepData(TestReproChangedData): stages = self.dvc.reproduce(file2_stage) - self.assertTrue(filecmp.cmp(self.file1, self.BAR)) - self.assertTrue(filecmp.cmp(file2, self.BAR)) + self.assertTrue(filecmp.cmp(self.file1, self.BAR, shallow=False)) + self.assertTrue(filecmp.cmp(file2, self.BAR, shallow=False)) self.assertEqual(len(stages), 3) @@ -175,8 +175,8 @@ class TestReproLocked(TestReproChangedData): ret = main(['unlock', file2_stage]) self.assertEqual(ret, 0) stages = self.dvc.reproduce(file2_stage) - self.assertTrue(filecmp.cmp(self.file1, self.BAR)) - self.assertTrue(filecmp.cmp(file2, self.BAR)) + self.assertTrue(filecmp.cmp(self.file1, self.BAR, shallow=False)) + self.assertTrue(filecmp.cmp(file2, self.BAR, shallow=False)) self.assertEqual(len(stages), 3) def test_non_existing(self): @@ -195,7 +195,7 @@ class TestReproPhony(TestReproChangedData): self.dvc.reproduce(stage.path) - self.assertTrue(filecmp.cmp(self.file1, self.BAR)) + self.assertTrue(filecmp.cmp(self.file1, self.BAR, shallow=False)) class TestNonExistingOutput(TestRepro): @@ -212,7 +212,7 @@ class TestReproDataSource(TestReproChangedData): stages = self.dvc.reproduce(self.foo_stage.path) - self.assertTrue(filecmp.cmp(self.FOO, self.BAR)) + self.assertTrue(filecmp.cmp(self.FOO, self.BAR, shallow=False)) self.assertEqual(stages[0].outs[0].md5, file_md5(self.BAR)[0]) @@ -293,7 +293,7 @@ class TestCmdReproChdir(TestDvc): self.assertEqual(ret, 0) self.assertTrue(os.path.isfile(foo)) self.assertTrue(os.path.isfile(bar)) - self.assertTrue(filecmp.cmp(foo, bar)) + self.assertTrue(filecmp.cmp(foo, bar, shallow=False)) os.unlink(bar) @@ -302,7 +302,7 @@ class TestCmdReproChdir(TestDvc): self.assertEqual(ret, 0) self.assertTrue(os.path.isfile(foo)) self.assertTrue(os.path.isfile(bar)) - self.assertTrue(filecmp.cmp(foo, bar)) + self.assertTrue(filecmp.cmp(foo, bar, shallow=False)) class TestReproExternalBase(TestDvc): @@ -361,7 +361,7 @@ class TestReproExternalBase(TestDvc): import_stage = self.dvc.imp(out_foo_path, 'import') self.assertTrue(os.path.exists('import')) - self.assertTrue(filecmp.cmp('import', self.FOO)) + self.assertTrue(filecmp.cmp('import', self.FOO, shallow=False)) import_remote_stage = self.dvc.imp(out_foo_path, out_foo_path + '_imported') @@ -376,7 +376,7 @@ class TestReproExternalBase(TestDvc): stages = self.dvc.reproduce(import_stage.path) self.assertEqual(len(stages), 1) self.assertTrue(os.path.exists('import')) - self.assertTrue(filecmp.cmp('import', self.BAR)) + self.assertTrue(filecmp.cmp('import', self.BAR, shallow=False)) stages = self.dvc.reproduce(cmd_stage.path) self.assertEqual(len(stages), 1) diff --git a/tests/test_run.py b/tests/test_run.py index f7088d125..587ff4351 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -26,7 +26,7 @@ class TestRun(TestDvc): fname=fname, cwd=cwd) - self.assertTrue(filecmp.cmp(self.FOO, 'out')) + self.assertTrue(filecmp.cmp(self.FOO, 'out', shallow=False)) self.assertTrue(os.path.isfile(stage.path)) self.assertEqual(stage.cmd, cmd) self.assertEqual(len(stage.deps), len(deps)) @@ -64,7 +64,7 @@ class TestCmdRun(TestDvc): self.assertEqual(ret, 0) self.assertTrue(os.path.isfile('out')) self.assertTrue(os.path.isfile('out.dvc')) - self.assertTrue(filecmp.cmp(self.FOO, 'out')) + self.assertTrue(filecmp.cmp(self.FOO, 'out', shallow=False)) def test_run_bad_command(self): ret = main(['run', diff --git a/tests/test_utils.py b/tests/test_utils.py index 045b803d0..d1a324822 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -18,10 +18,10 @@ class TestUtils(TestDvc): os.mkdir(dest_dir) utils.copyfile(src, dest) - self.assertTrue(filecmp.cmp(src, dest)) + self.assertTrue(filecmp.cmp(src, dest, shallow=False)) utils.copyfile(src, dest_dir) - self.assertTrue(filecmp.cmp(src, '{}/{}'.format(dest_dir, src))) + self.assertTrue(filecmp.cmp(src, '{}/{}'.format(dest_dir, src), shallow=False)) shutil.rmtree(dest_dir) os.remove(src)