From e11c3cbbc5a6743db4025c4ee5ae9a58a4c6b258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20P=C5=99=C3=ADhoda?= Date: Tue, 21 May 2019 10:21:54 +0200 Subject: [PATCH 1/2] Add repro -R, support dirs & stage files when running pull/push/repro -R. --- dvc/command/repro.py | 11 ++- dvc/repo/__init__.py | 4 +- dvc/repo/reproduce.py | 26 +++-- tests/func/test_checkout.py | 6 +- tests/func/test_repro.py | 165 +++++++++++++++++++++++++++++++ tests/unit/command/test_repro.py | 3 +- 6 files changed, 199 insertions(+), 16 deletions(-) diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 2d3dfba9f..271d6908d 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -15,7 +15,6 @@ logger = logging.getLogger(__name__) class CmdRepro(CmdBase): def run(self): - recursive = not self.args.single_item saved_dir = os.path.realpath(os.curdir) if self.args.cwd: os.chdir(self.args.cwd) @@ -31,7 +30,7 @@ class CmdRepro(CmdBase): try: stages = self.repo.reproduce( target, - recursive=recursive, + single_item=self.args.single_item, force=self.args.force, dry=self.args.dry, interactive=self.args.interactive, @@ -40,6 +39,7 @@ class CmdRepro(CmdBase): ignore_build_cache=self.args.ignore_build_cache, no_commit=self.args.no_commit, downstream=self.args.downstream, + recursive=self.args.recursive, ) if len(stages) == 0: @@ -128,6 +128,13 @@ def add_parser(subparsers, parent_parser): default=False, help="Reproduce all pipelines in the repo.", ) + repro_parser.add_argument( + "-R", + "--recursive", + action="store_true", + default=False, + help="Reproduce all stages in the specified directory.", + ) repro_parser.add_argument( "--ignore-build-cache", action="store_true", diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 94cb0e879..beb188cfc 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -157,7 +157,7 @@ class Repo(object): import networkx as nx from dvc.stage import Stage - if not target or recursive: + if not target or (recursive and os.path.isdir(target)): return self.active_stages(target) stage = Stage.load(self, target) @@ -305,7 +305,7 @@ class Repo(object): all_branches=all_branches, all_tags=all_tags ): if target: - if recursive: + if recursive and os.path.isdir(target): stages = self.stages(target) else: stages = self.collect(target, with_deps=with_deps) diff --git a/dvc/repo/reproduce.py b/dvc/repo/reproduce.py index 672d03f76..4a7b51ce7 100644 --- a/dvc/repo/reproduce.py +++ b/dvc/repo/reproduce.py @@ -35,7 +35,7 @@ def _reproduce_stage(stages, node, force, dry, interactive, no_commit): def reproduce( self, target=None, - recursive=True, + single_item=False, force=False, dry=False, interactive=False, @@ -44,7 +44,9 @@ def reproduce( ignore_build_cache=False, no_commit=False, downstream=False, + recursive=False, ): + import networkx as nx from dvc.stage import Stage if not target and not all_pipelines: @@ -56,7 +58,13 @@ def reproduce( interactive = core.get(config.SECTION_CORE_INTERACTIVE, False) targets = [] - if pipeline or all_pipelines: + if recursive and os.path.isdir(target): + G = self.graph(from_directory=target)[1] + dir_targets = [ + os.path.join(self.root_dir, n) for n in nx.dfs_postorder_nodes(G) + ] + targets.extend(dir_targets) + elif pipeline or all_pipelines: if pipeline: stage = Stage.load(self, target) node = os.path.relpath(stage.path, self.root_dir) @@ -77,7 +85,7 @@ def reproduce( stages = _reproduce( self, target, - recursive=recursive, + single_item=single_item, force=force, dry=dry, interactive=interactive, @@ -93,7 +101,7 @@ def reproduce( def _reproduce( self, target, - recursive=True, + single_item=False, force=False, dry=False, interactive=False, @@ -109,7 +117,11 @@ def _reproduce( stages = nx.get_node_attributes(G, "stage") node = os.path.relpath(stage.path, self.root_dir) - if recursive: + if single_item: + ret = _reproduce_stage( + stages, node, force, dry, interactive, no_commit + ) + else: ret = _reproduce_stages( G, stages, @@ -121,10 +133,6 @@ def _reproduce( no_commit, downstream, ) - else: - ret = _reproduce_stage( - stages, node, force, dry, interactive, no_commit - ) return ret diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py index d6f1f5169..b9ed9d264 100644 --- a/tests/func/test_checkout.py +++ b/tests/func/test_checkout.py @@ -517,8 +517,10 @@ class TestCheckoutRecursiveNotDirectory(TestDvc): ret = main(["add", self.FOO]) self.assertEqual(0, ret) - with self.assertRaises(TargetNotDirectoryError): - self.dvc.checkout(target=self.FOO, recursive=True) + try: + self.dvc.checkout(target=self.FOO + ".dvc", recursive=True) + except TargetNotDirectoryError: + self.fail("should not raise TargetNotDirectoryError") class TestCheckoutMovedCacheDirWithSymlinks(TestDvc): diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index dad983adc..9e87a6eb3 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -1370,6 +1370,171 @@ class TestShouldDisplayMetricsOnReproWithMetricsOption(TestDvc): self.assertIn(expected_metrics_display, self._caplog.text) +@pytest.fixture +def repro_dir(dvc, repo_dir): + repo_dir.dname = "dir" + os.mkdir(repo_dir.dname) + repo_dir.emptydname = "emptydir" + os.mkdir(repo_dir.emptydname) + subdname = os.path.join(repo_dir.dname, "subdir") + os.mkdir(subdname) + + repo_dir.source = "source" + repo_dir.source_stage = repo_dir.source + ".dvc" + stage = dvc.run( + fname=repo_dir.source_stage, + outs=[repo_dir.source], + deps=[repo_dir.FOO], + ) + assert stage is not None + assert filecmp.cmp(repo_dir.source, repo_dir.FOO, shallow=False) + + repo_dir.unrelated1 = "unrelated1" + repo_dir.unrelated1_stage = repo_dir.unrelated1 + ".dvc" + stage = dvc.run( + fname=repo_dir.unrelated1_stage, + outs=[repo_dir.unrelated1], + deps=[repo_dir.source], + ) + assert stage is not None + + repo_dir.unrelated2 = "unrelated2" + repo_dir.unrelated2_stage = repo_dir.unrelated2 + ".dvc" + stage = dvc.run( + fname=repo_dir.unrelated2_stage, + outs=[repo_dir.unrelated2], + deps=[repo_dir.DATA], + ) + assert stage is not None + + repo_dir.first = os.path.join(repo_dir.dname, "first") + repo_dir.first_stage = repo_dir.first + ".dvc" + + stage = dvc.run( + fname=repo_dir.first_stage, + deps=[repo_dir.source], + outs=[repo_dir.first], + cmd="python {} {} {}".format( + repo_dir.CODE, repo_dir.source, repo_dir.first + ), + ) + assert stage is not None + assert filecmp.cmp(repo_dir.first, repo_dir.FOO, shallow=False) + + repo_dir.second = os.path.join(subdname, "second") + repo_dir.second_stage = repo_dir.second + ".dvc" + stage = dvc.run( + fname=repo_dir.second_stage, + outs=[repo_dir.second], + deps=[repo_dir.DATA], + ) + assert stage is not None + assert filecmp.cmp(repo_dir.second, repo_dir.DATA, shallow=False) + + repo_dir.third_stage = os.path.join(repo_dir.dname, "Dvcfile") + stage = dvc.run( + fname=repo_dir.third_stage, deps=[repo_dir.first, repo_dir.second] + ) + assert stage is not None + + yield repo_dir + + +def test_recursive_repro_default(dvc, repro_dir): + """ + Test recursive repro on dir after a dep outside this dir has changed. + """ + os.unlink(repro_dir.FOO) + shutil.copyfile(repro_dir.BAR, repro_dir.FOO) + + stages = dvc.reproduce(repro_dir.dname, recursive=True) + # Check that the dependency ("source") and the dependent stages + # inside the folder have been reproduced ("first", "third") + assert len(stages) == 3 + names = [stage.relpath for stage in stages] + assert repro_dir.source_stage in names + assert repro_dir.first_stage in names + assert repro_dir.third_stage in names + assert filecmp.cmp(repro_dir.source, repro_dir.BAR, shallow=False) + assert filecmp.cmp(repro_dir.first, repro_dir.BAR, shallow=False) + + +def test_recursive_repro_single(dvc, repro_dir): + """ + Test recursive single-item repro on dir + after a dep outside this dir has changed. + """ + os.unlink(repro_dir.FOO) + shutil.copyfile(repro_dir.BAR, repro_dir.FOO) + + os.unlink(repro_dir.DATA) + shutil.copyfile(repro_dir.BAR, repro_dir.DATA) + + stages = dvc.reproduce(repro_dir.dname, recursive=True, single_item=True) + # Check that just stages inside given dir + # with changed direct deps have been reproduced. + # This means that "first" stage should not be reproduced + # since it depends on "source". + # Also check that "second" stage was reproduced before "third" stage + assert len(stages) == 2 + assert repro_dir.second_stage == stages[0].relpath + assert repro_dir.third_stage == stages[1].relpath + assert filecmp.cmp(repro_dir.second, repro_dir.BAR, shallow=False) + + +def test_recursive_repro_single_force(dvc, repro_dir): + """ + Test recursive single-item force repro on dir + without any dependencies changing. + """ + stages = dvc.reproduce( + repro_dir.dname, recursive=True, single_item=True, force=True + ) + assert len(stages) == 3 + names = [stage.relpath for stage in stages] + # Check that all stages inside given dir have been reproduced + # Also check that "second" stage was reproduced before "third" stage + # and that "first" stage was reproduced before "third" stage + assert repro_dir.first_stage in names + assert repro_dir.second_stage in names + assert repro_dir.third_stage in names + assert names.index(repro_dir.first_stage) < names.index( + repro_dir.third_stage + ) + assert names.index(repro_dir.second_stage) < names.index( + repro_dir.third_stage + ) + + +def test_recursive_repro_empty_dir(dvc, repro_dir): + """ + Test recursive repro on an empty directory + """ + stages = dvc.reproduce(repro_dir.emptydname, recursive=True, force=True) + assert len(stages) == 0 + + +def test_recursive_repro_recursive_missing_file(dvc): + """ + Test recursive repro on a missing file + """ + with pytest.raises(StageFileDoesNotExistError): + dvc.reproduce("notExistingStage.dvc", recursive=True) + with pytest.raises(StageFileDoesNotExistError): + dvc.reproduce("notExistingDir/", recursive=True) + + +def test_recursive_repro_on_stage_file(dvc, repro_dir): + """ + Test recursive repro on a stage file instead of directory + """ + stages = dvc.reproduce(repro_dir.first_stage, recursive=True, force=True) + assert len(stages) == 2 + names = [stage.relpath for stage in stages] + assert repro_dir.source_stage in names + assert repro_dir.first_stage in names + + @pytest.fixture def foo_copy(repo_dir, dvc_repo): stages = dvc_repo.add(repo_dir.FOO) diff --git a/tests/unit/command/test_repro.py b/tests/unit/command/test_repro.py index b0c08ea58..4e9688e87 100644 --- a/tests/unit/command/test_repro.py +++ b/tests/unit/command/test_repro.py @@ -10,7 +10,8 @@ default_arguments = { "interactive": False, "no_commit": False, "pipeline": False, - "recursive": True, + "single_item": False, + "recursive": False, } From d072c2bd79ead810e6a1f7892dbf4e06685fd118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20P=C5=99=C3=ADhoda?= Date: Mon, 27 May 2019 14:14:44 +0200 Subject: [PATCH 2/2] Fix after renaming dvc fixture to dvc_repo. --- tests/func/test_repro.py | 46 +++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 9e87a6eb3..a58fcbd6f 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -1371,7 +1371,7 @@ class TestShouldDisplayMetricsOnReproWithMetricsOption(TestDvc): @pytest.fixture -def repro_dir(dvc, repo_dir): +def repro_dir(dvc_repo, repo_dir): repo_dir.dname = "dir" os.mkdir(repo_dir.dname) repo_dir.emptydname = "emptydir" @@ -1381,7 +1381,7 @@ def repro_dir(dvc, repo_dir): repo_dir.source = "source" repo_dir.source_stage = repo_dir.source + ".dvc" - stage = dvc.run( + stage = dvc_repo.run( fname=repo_dir.source_stage, outs=[repo_dir.source], deps=[repo_dir.FOO], @@ -1391,7 +1391,7 @@ def repro_dir(dvc, repo_dir): repo_dir.unrelated1 = "unrelated1" repo_dir.unrelated1_stage = repo_dir.unrelated1 + ".dvc" - stage = dvc.run( + stage = dvc_repo.run( fname=repo_dir.unrelated1_stage, outs=[repo_dir.unrelated1], deps=[repo_dir.source], @@ -1400,7 +1400,7 @@ def repro_dir(dvc, repo_dir): repo_dir.unrelated2 = "unrelated2" repo_dir.unrelated2_stage = repo_dir.unrelated2 + ".dvc" - stage = dvc.run( + stage = dvc_repo.run( fname=repo_dir.unrelated2_stage, outs=[repo_dir.unrelated2], deps=[repo_dir.DATA], @@ -1410,7 +1410,7 @@ def repro_dir(dvc, repo_dir): repo_dir.first = os.path.join(repo_dir.dname, "first") repo_dir.first_stage = repo_dir.first + ".dvc" - stage = dvc.run( + stage = dvc_repo.run( fname=repo_dir.first_stage, deps=[repo_dir.source], outs=[repo_dir.first], @@ -1423,7 +1423,7 @@ def repro_dir(dvc, repo_dir): repo_dir.second = os.path.join(subdname, "second") repo_dir.second_stage = repo_dir.second + ".dvc" - stage = dvc.run( + stage = dvc_repo.run( fname=repo_dir.second_stage, outs=[repo_dir.second], deps=[repo_dir.DATA], @@ -1432,7 +1432,7 @@ def repro_dir(dvc, repo_dir): assert filecmp.cmp(repo_dir.second, repo_dir.DATA, shallow=False) repo_dir.third_stage = os.path.join(repo_dir.dname, "Dvcfile") - stage = dvc.run( + stage = dvc_repo.run( fname=repo_dir.third_stage, deps=[repo_dir.first, repo_dir.second] ) assert stage is not None @@ -1440,14 +1440,14 @@ def repro_dir(dvc, repo_dir): yield repo_dir -def test_recursive_repro_default(dvc, repro_dir): +def test_recursive_repro_default(dvc_repo, repro_dir): """ Test recursive repro on dir after a dep outside this dir has changed. """ os.unlink(repro_dir.FOO) shutil.copyfile(repro_dir.BAR, repro_dir.FOO) - stages = dvc.reproduce(repro_dir.dname, recursive=True) + stages = dvc_repo.reproduce(repro_dir.dname, recursive=True) # Check that the dependency ("source") and the dependent stages # inside the folder have been reproduced ("first", "third") assert len(stages) == 3 @@ -1459,7 +1459,7 @@ def test_recursive_repro_default(dvc, repro_dir): assert filecmp.cmp(repro_dir.first, repro_dir.BAR, shallow=False) -def test_recursive_repro_single(dvc, repro_dir): +def test_recursive_repro_single(dvc_repo, repro_dir): """ Test recursive single-item repro on dir after a dep outside this dir has changed. @@ -1470,7 +1470,9 @@ def test_recursive_repro_single(dvc, repro_dir): os.unlink(repro_dir.DATA) shutil.copyfile(repro_dir.BAR, repro_dir.DATA) - stages = dvc.reproduce(repro_dir.dname, recursive=True, single_item=True) + stages = dvc_repo.reproduce( + repro_dir.dname, recursive=True, single_item=True + ) # Check that just stages inside given dir # with changed direct deps have been reproduced. # This means that "first" stage should not be reproduced @@ -1482,12 +1484,12 @@ def test_recursive_repro_single(dvc, repro_dir): assert filecmp.cmp(repro_dir.second, repro_dir.BAR, shallow=False) -def test_recursive_repro_single_force(dvc, repro_dir): +def test_recursive_repro_single_force(dvc_repo, repro_dir): """ Test recursive single-item force repro on dir without any dependencies changing. """ - stages = dvc.reproduce( + stages = dvc_repo.reproduce( repro_dir.dname, recursive=True, single_item=True, force=True ) assert len(stages) == 3 @@ -1506,29 +1508,33 @@ def test_recursive_repro_single_force(dvc, repro_dir): ) -def test_recursive_repro_empty_dir(dvc, repro_dir): +def test_recursive_repro_empty_dir(dvc_repo, repro_dir): """ Test recursive repro on an empty directory """ - stages = dvc.reproduce(repro_dir.emptydname, recursive=True, force=True) + stages = dvc_repo.reproduce( + repro_dir.emptydname, recursive=True, force=True + ) assert len(stages) == 0 -def test_recursive_repro_recursive_missing_file(dvc): +def test_recursive_repro_recursive_missing_file(dvc_repo): """ Test recursive repro on a missing file """ with pytest.raises(StageFileDoesNotExistError): - dvc.reproduce("notExistingStage.dvc", recursive=True) + dvc_repo.reproduce("notExistingStage.dvc", recursive=True) with pytest.raises(StageFileDoesNotExistError): - dvc.reproduce("notExistingDir/", recursive=True) + dvc_repo.reproduce("notExistingDir/", recursive=True) -def test_recursive_repro_on_stage_file(dvc, repro_dir): +def test_recursive_repro_on_stage_file(dvc_repo, repro_dir): """ Test recursive repro on a stage file instead of directory """ - stages = dvc.reproduce(repro_dir.first_stage, recursive=True, force=True) + stages = dvc_repo.reproduce( + repro_dir.first_stage, recursive=True, force=True + ) assert len(stages) == 2 names = [stage.relpath for stage in stages] assert repro_dir.source_stage in names