From fba7d85cbd053a4dcabf9eae65cd7a45eea89fb0 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Mon, 10 Jun 2019 18:08:28 -0500 Subject: [PATCH 01/13] dvc: Use "DVC-file" term as much as possible Keep "stage file" in the context of pipelines No issue --- dvc/exceptions.py | 6 +- dvc/repo/add.py | 2 +- dvc/repo/metrics/show.py | 2 +- dvc/repo/move.py | 2 +- dvc/repo/pkg/install.py | 173 +++++++++++++++++++++++++++++++++++++++ dvc/stage.py | 20 ++--- tests/func/test_add.py | 8 +- tests/func/test_repro.py | 2 +- tests/func/test_stage.py | 2 +- 9 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 dvc/repo/pkg/install.py diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 870dbd542..c5496fa4e 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -62,7 +62,7 @@ class OutputNotFoundError(DvcException): def __init__(self, output): super(OutputNotFoundError, self).__init__( - "unable to find stage file with output '{path}'".format( + "unable to find DVC-file with output '{path}'".format( path=relpath(output) ) ) @@ -74,7 +74,7 @@ class StagePathAsOutputError(DvcException): Args: cwd (str): path to the directory. - fname (str): path to the stage file that has cwd specified as an + fname (str): path to the DVC-file that has cwd specified as an output. """ @@ -216,7 +216,7 @@ class StageFileCorruptedError(DvcException): def __init__(self, path, cause=None): path = relpath(path) super(StageFileCorruptedError, self).__init__( - "unable to read stage file: {} " + "unable to read DVC-file: {} " "YAML file structure is corrupted".format(path), cause=cause, ) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 2cb3dad0c..0b02a19ab 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -22,7 +22,7 @@ def add(repo, target, recursive=False, no_commit=False, fname=None): logger.warning( "You are adding a large directory '{target}' recursively," " consider tracking it as a whole instead.\n" - "{purple}HINT:{nc} Remove the generated stage files and then" + "{purple}HINT:{nc} Remove the generated DVC-file and then" " run {cyan}dvc add {target}{nc}".format( purple=colorama.Fore.MAGENTA, cyan=colorama.Fore.CYAN, diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 6ce9f8e71..758ba3a47 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -177,7 +177,7 @@ def _collect_metrics(repo, path, recursive, typ, xpath, branch): outs = repo.find_outs_by_path(path, outs=outs, recursive=recursive) except OutputNotFoundError: logger.debug( - "stage file not for found for '{}' in branch '{}'".format( + "DVC-file not for found for '{}' in branch '{}'".format( path, branch ) ) diff --git a/dvc/repo/move.py b/dvc/repo/move.py index 7b5dd6c61..ea190fe61 100644 --- a/dvc/repo/move.py +++ b/dvc/repo/move.py @@ -17,7 +17,7 @@ def move(self, from_path, to_path): to reflect the change on the pipeline. If the output has the same name as its stage, it would - also rename the corresponding stage file. + also rename the corresponding DVC-file. E.g. Having: (hello, hello.dvc) diff --git a/dvc/repo/pkg/install.py b/dvc/repo/pkg/install.py new file mode 100644 index 000000000..5a17ead84 --- /dev/null +++ b/dvc/repo/pkg/install.py @@ -0,0 +1,173 @@ +from __future__ import unicode_literals + +import os +import shutil +import logging + +from dvc.exceptions import DvcException +from dvc.stage import Stage +from dvc.scm.git.temp_git_repo import TempGitRepo + + +logger = logging.getLogger(__name__) + + +class PackageManager(object): + PACKAGE_FILE = "package.yaml" + + @staticmethod + def read_packages(): + return [] + + @staticmethod + def get_package(addr): + for pkg_class in [GitPackage]: + try: + return pkg_class() + except Exception: + pass + return None + + def __init__(self, addr): + self._addr = addr + + +class Package(object): + MODULES_DIR = "dvc_mod" + + def install_or_update( + self, parent_repo, address, target_dir, select=[], fname=None + ): + raise NotImplementedError( + "A method of abstract Package class was called" + ) + + def is_in_root(self): + return True + + +class GitPackage(Package): + DEF_DVC_FILE_PREFIX = "mod_" + + def install_or_update( + self, parent_repo, address, target_dir, select=[], fname=None + ): + from git.cmd import Git + + if not self.is_in_root(): + raise DvcException( + "This command can be run only from a repository root" + ) + + if not os.path.exists(self.MODULES_DIR): + logger.debug("Creating modules dir {}".format(self.MODULES_DIR)) + os.makedirs(self.MODULES_DIR) + parent_repo.scm.ignore(os.path.abspath(self.MODULES_DIR)) + + module_name = Git.polish_url(address).strip("/").split("/")[-1] + if not module_name: + raise DvcException( + "Package address error: unable to extract package name" + ) + + with TempGitRepo( + address, module_name, Package.MODULES_DIR + ) as tmp_repo: + outputs_to_copy = tmp_repo.outs + if select: + outputs_to_copy = list( + filter(lambda out: out.dvc_path in select, outputs_to_copy) + ) + + fetched_stage_files = set( + map(lambda o: o.stage.path, outputs_to_copy) + ) + tmp_repo.fetch(fetched_stage_files) + + module_dir = self.create_module_dir(module_name) + tmp_repo.persist_to(module_dir, parent_repo) + + dvc_file = self.get_dvc_file_name(fname, target_dir, module_name) + try: + self.persist_stage_and_scm_state( + parent_repo, outputs_to_copy, target_dir, dvc_file + ) + except Exception as ex: + raise DvcException( + "Package '{}' was installed " + "but DVC-file '{}' " + "was not created properly: {}".format( + address, dvc_file, ex + ) + ) + + parent_repo.checkout(dvc_file) + + @staticmethod + def persist_stage_and_scm_state( + parent_repo, outputs_to_copy, target_dir, dvc_file + ): + stage = Stage.create( + repo=parent_repo, + fname=dvc_file, + validate_state=False, + wdir=target_dir, + ) + stage.outs = list( + map(lambda o: o.assign_to_stage_file(stage), outputs_to_copy) + ) + + for out in stage.outs: + parent_repo.scm.ignore(out.fspath, in_curr_dir=True) + + stage.dump() + + @staticmethod + def create_module_dir(module_name): + module_dir = os.path.join(GitPackage.MODULES_DIR, module_name) + if os.path.exists(module_dir): + logger.info("Updating package {}".format(module_name)) + shutil.rmtree(module_dir) + else: + logger.info("Adding package {}".format(module_name)) + return module_dir + + def get_dvc_file_name(self, stage_file, target_dir, module_name): + if stage_file: + dvc_file_path = stage_file + else: + dvc_file_name = self.DEF_DVC_FILE_PREFIX + module_name + ".dvc" + dvc_file_path = os.path.join(target_dir, dvc_file_name) + return dvc_file_path + + +def install(self, address, target_dir, select=[], fname=None): + """ + Install package. + + The command can be run only from DVC project root. + + E.g. + Having: DVC package in https://github.com/dmpetrov/tag_classifier + + $ dvc pkg install https://github.com/dmpetrov/tag_classifier + + Result: tag_classifier package in dvc_mod/ directory + """ + + if not os.path.isdir(target_dir): + raise DvcException( + "target directory '{}' does not exist".format(target_dir) + ) + + curr_dir = os.path.realpath(os.curdir) + if not os.path.realpath(target_dir).startswith(curr_dir): + raise DvcException( + "the current directory should be a subdirectory of the target " + "dir '{}'".format(target_dir) + ) + + addresses = [address] if address else PackageManager.read_packages() + for addr in addresses: + mgr = PackageManager.get_package(addr) + mgr.install_or_update(self, address, target_dir, select, fname) diff --git a/dvc/stage.py b/dvc/stage.py index 5abc53e54..285a12336 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -35,7 +35,7 @@ class StageCmdFailedError(DvcException): class StageFileFormatError(DvcException): def __init__(self, fname, e): - msg = "stage file '{}' format error: {}".format(fname, str(e)) + msg = "DVC-file '{}' format error: {}".format(fname, str(e)) super(StageFileFormatError, self).__init__(msg) @@ -183,7 +183,7 @@ class Stage(object): @property def is_data_source(self): - """Whether the stage file was created with `dvc add` or `dvc import`""" + """Whether the DVC-file was created with `dvc add` or `dvc import`""" return self.cmd is None @staticmethod @@ -206,14 +206,14 @@ class Stage(object): @property def is_callback(self): """ - A callback stage is always considered as changed, + An orphan stage is always considered as changed, so it runs on every `dvc repro` call. """ return not self.is_data_source and len(self.deps) == 0 @property def is_import(self): - """Whether the stage file was created with `dvc import`.""" + """Whether the DVC-file was created with `dvc import`.""" return not self.cmd and len(self.deps) == 1 and len(self.outs) == 1 @property @@ -229,7 +229,7 @@ class Stage(object): if self.is_callback: logger.warning( - "DVC-file '{fname}' is a 'callback' stage " + "DVC-file '{fname}' is an 'orphan' stage " "(has a command and no dependencies) and thus always " "considered as changed.".format(fname=self.relpath) ) @@ -453,9 +453,9 @@ class Stage(object): if wdir is None and cwd is not None: if fname is not None and os.path.basename(fname) != fname: raise StageFileBadNameError( - "stage file name '{fname}' may not contain subdirectories" + "DVC-file name '{fname}' may not contain subdirectories" " if '-c|--cwd' (deprecated) is specified. Use '-w|--wdir'" - " along with '-f' to specify stage file path and working" + " along with '-f' to specify DVC-file path with working" " directory.".format(fname=fname) ) wdir = cwd @@ -559,7 +559,7 @@ class Stage(object): def _check_dvc_filename(fname): if not Stage.is_valid_filename(fname): raise StageFileBadNameError( - "bad stage filename '{}'. Stage files should be named" + "bad DVC-file name '{}'. DVC-files should be named" " 'Dvcfile' or have a '.dvc' suffix (e.g. '{}.dvc').".format( relpath(fname), os.path.basename(fname) ) @@ -663,10 +663,10 @@ class Stage(object): if self.PARAM_MD5 in d.keys(): del d[self.PARAM_MD5] - # Ignore the wdir default value. In this case stage file w/o + # Ignore the wdir default value. In this case DVC-file w/o # wdir has the same md5 as a file with the default value specified. # It's important for backward compatibility with pipelines that - # didn't have WDIR in their stage files. + # didn't have WDIR in their DVC-files. if d.get(self.PARAM_WDIR) == ".": del d[self.PARAM_WDIR] diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 122a06ec0..8ad46ab90 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -96,7 +96,7 @@ class TestAddCmdDirectoryRecursive(TestDvc): warning = ( "You are adding a large directory 'large-dir' recursively," " consider tracking it as a whole instead.\n" - "{purple}HINT:{nc} Remove the generated stage files and then" + "{purple}HINT:{nc} Remove the generated DVC-files and then" " run {cyan}dvc add large-dir{nc}".format( purple=colorama.Fore.MAGENTA, cyan=colorama.Fore.CYAN, @@ -424,7 +424,7 @@ class TestShouldThrowProperExceptionOnCorruptedStageFile(TestDvc): foo_stage = relpath(self.FOO + Stage.STAGE_FILE_SUFFIX) - # corrupt stage file + # corrupt DVC-file with open(foo_stage, "a+") as file: file.write("this will break yaml file structure") @@ -434,7 +434,7 @@ class TestShouldThrowProperExceptionOnCorruptedStageFile(TestDvc): assert 1 == ret expected_error = ( - "unable to read stage file: {} " + "unable to read DVC-file: {} " "YAML file structure is corrupted".format(foo_stage) ) @@ -475,7 +475,7 @@ class TestShouldCleanUpAfterFailedAdd(TestDvcGit): self.assertEqual(0, ret) foo_stage_file = self.FOO + Stage.STAGE_FILE_SUFFIX - # corrupt stage file + # corrupt DVC-file with open(foo_stage_file, "a+") as file: file.write("this will break yaml file structure") diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index d01dfd656..1185d733c 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -1558,7 +1558,7 @@ def test_dvc_formatting_retained(dvc_repo, foo_copy): root = pathlib.Path(dvc_repo.root_dir) stage_file = root / foo_copy["stage_fname"] - # Add comments and custom formatting to stage file + # Add comments and custom formatting to DVC-file lines = list(map(_format_dvc_line, stage_file.read_text().splitlines())) lines.insert(0, "# Starting comment") stage_text = "".join(l + "\n" for l in lines) diff --git a/tests/func/test_stage.py b/tests/func/test_stage.py index 6c56a1a53..fa18160d8 100644 --- a/tests/func/test_stage.py +++ b/tests/func/test_stage.py @@ -172,7 +172,7 @@ def test_md5_ignores_comments(repo_dir, dvc_repo): def test_meta_is_preserved(dvc_repo): stage, = dvc_repo.add("foo") - # Add meta to stage file + # Add meta to DVC-file data = load_stage_file(stage.path) data["meta"] = {"custom_key": 42} dump_stage_file(stage.path, data) From f004a84e7cfcb23fa267f406c482602c891d3fda Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Sat, 22 Jun 2019 00:23:25 -0500 Subject: [PATCH 02/13] tests: fix failing test_add (due to extra 's' in word) To close #2112 --- tests/func/test_add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 8ad46ab90..ce3a8fecd 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -96,7 +96,7 @@ class TestAddCmdDirectoryRecursive(TestDvc): warning = ( "You are adding a large directory 'large-dir' recursively," " consider tracking it as a whole instead.\n" - "{purple}HINT:{nc} Remove the generated DVC-files and then" + "{purple}HINT:{nc} Remove the generated DVC-file and then" " run {cyan}dvc add large-dir{nc}".format( purple=colorama.Fore.MAGENTA, cyan=colorama.Fore.CYAN, From 5b2405f3bee58e4eb8af894d1534b6bbc75039b6 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Sat, 22 Jun 2019 23:51:46 -0500 Subject: [PATCH 03/13] scripts: fix zsh autocomplete for `dvc pipeline` subcommands --- scripts/completion/dvc.zsh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/completion/dvc.zsh b/scripts/completion/dvc.zsh index 0faddee63..8bcd382ca 100644 --- a/scripts/completion/dvc.zsh +++ b/scripts/completion/dvc.zsh @@ -144,7 +144,7 @@ _dvc_move=( ) _dvc_pipeline=( - "1:Sub command:(show,list)" + "1:Sub command:(show list)" ) _dvc_pull=( From 79b22fc7767b93acfc61f93791848bfb638e7aeb Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Tue, 25 Jun 2019 13:11:53 -0500 Subject: [PATCH 04/13] help: fixed -h output for `diff` and `remote` subcommands Fix #2169 --- dvc/command/diff.py | 2 +- dvc/command/remote.py | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 901223d04..b62ef1c29 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -157,7 +157,7 @@ def add_parser(subparsers, parent_parser): diff_parser.add_argument( "b_ref", help=( - "Git reference untill which diff calculates, if omitted " + "Git reference until which diff calculates, if omitted " "diff shows the difference between current HEAD and a_ref" ), nargs="?", diff --git a/dvc/command/remote.py b/dvc/command/remote.py index 4e0415556..490bbbe48 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -89,7 +89,7 @@ def add_parser(subparsers, parent_parser): help=REMOTE_ADD_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - remote_add_parser.add_argument("name", help="Name.") + remote_add_parser.add_argument("name", help="Name of the remote") remote_add_parser.add_argument( "url", help="URL. See full list of supported urls at " "man.dvc.org/remote", @@ -138,7 +138,9 @@ def add_parser(subparsers, parent_parser): help=REMOTE_REMOVE_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - remote_remove_parser.add_argument("name", help="Name") + remote_remove_parser.add_argument( + "name", help="Name of the remote to remove." + ) remote_remove_parser.set_defaults(func=CmdRemoteRemove) REMOTE_MODIFY_HELP = "Modify remote." @@ -149,9 +151,13 @@ def add_parser(subparsers, parent_parser): help=REMOTE_MODIFY_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - remote_modify_parser.add_argument("name", help="Name.") - remote_modify_parser.add_argument("option", help="Option.") - remote_modify_parser.add_argument("value", nargs="?", help="Value.") + remote_modify_parser.add_argument("name", help="Name of the remote.") + remote_modify_parser.add_argument( + "option", help="Name of the option to modify." + ) + remote_modify_parser.add_argument( + "value", nargs="?", help="(optional) Value of the option." + ) remote_modify_parser.add_argument( "-u", "--unset", From bac1e60e497868be3033f82ec05497852a51466b Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 26 Jun 2019 14:10:27 -0500 Subject: [PATCH 05/13] pkg: rm dvc/repo/pkg/install.py For #2112 --- dvc/repo/pkg/install.py | 173 ---------------------------------------- 1 file changed, 173 deletions(-) delete mode 100644 dvc/repo/pkg/install.py diff --git a/dvc/repo/pkg/install.py b/dvc/repo/pkg/install.py deleted file mode 100644 index 5a17ead84..000000000 --- a/dvc/repo/pkg/install.py +++ /dev/null @@ -1,173 +0,0 @@ -from __future__ import unicode_literals - -import os -import shutil -import logging - -from dvc.exceptions import DvcException -from dvc.stage import Stage -from dvc.scm.git.temp_git_repo import TempGitRepo - - -logger = logging.getLogger(__name__) - - -class PackageManager(object): - PACKAGE_FILE = "package.yaml" - - @staticmethod - def read_packages(): - return [] - - @staticmethod - def get_package(addr): - for pkg_class in [GitPackage]: - try: - return pkg_class() - except Exception: - pass - return None - - def __init__(self, addr): - self._addr = addr - - -class Package(object): - MODULES_DIR = "dvc_mod" - - def install_or_update( - self, parent_repo, address, target_dir, select=[], fname=None - ): - raise NotImplementedError( - "A method of abstract Package class was called" - ) - - def is_in_root(self): - return True - - -class GitPackage(Package): - DEF_DVC_FILE_PREFIX = "mod_" - - def install_or_update( - self, parent_repo, address, target_dir, select=[], fname=None - ): - from git.cmd import Git - - if not self.is_in_root(): - raise DvcException( - "This command can be run only from a repository root" - ) - - if not os.path.exists(self.MODULES_DIR): - logger.debug("Creating modules dir {}".format(self.MODULES_DIR)) - os.makedirs(self.MODULES_DIR) - parent_repo.scm.ignore(os.path.abspath(self.MODULES_DIR)) - - module_name = Git.polish_url(address).strip("/").split("/")[-1] - if not module_name: - raise DvcException( - "Package address error: unable to extract package name" - ) - - with TempGitRepo( - address, module_name, Package.MODULES_DIR - ) as tmp_repo: - outputs_to_copy = tmp_repo.outs - if select: - outputs_to_copy = list( - filter(lambda out: out.dvc_path in select, outputs_to_copy) - ) - - fetched_stage_files = set( - map(lambda o: o.stage.path, outputs_to_copy) - ) - tmp_repo.fetch(fetched_stage_files) - - module_dir = self.create_module_dir(module_name) - tmp_repo.persist_to(module_dir, parent_repo) - - dvc_file = self.get_dvc_file_name(fname, target_dir, module_name) - try: - self.persist_stage_and_scm_state( - parent_repo, outputs_to_copy, target_dir, dvc_file - ) - except Exception as ex: - raise DvcException( - "Package '{}' was installed " - "but DVC-file '{}' " - "was not created properly: {}".format( - address, dvc_file, ex - ) - ) - - parent_repo.checkout(dvc_file) - - @staticmethod - def persist_stage_and_scm_state( - parent_repo, outputs_to_copy, target_dir, dvc_file - ): - stage = Stage.create( - repo=parent_repo, - fname=dvc_file, - validate_state=False, - wdir=target_dir, - ) - stage.outs = list( - map(lambda o: o.assign_to_stage_file(stage), outputs_to_copy) - ) - - for out in stage.outs: - parent_repo.scm.ignore(out.fspath, in_curr_dir=True) - - stage.dump() - - @staticmethod - def create_module_dir(module_name): - module_dir = os.path.join(GitPackage.MODULES_DIR, module_name) - if os.path.exists(module_dir): - logger.info("Updating package {}".format(module_name)) - shutil.rmtree(module_dir) - else: - logger.info("Adding package {}".format(module_name)) - return module_dir - - def get_dvc_file_name(self, stage_file, target_dir, module_name): - if stage_file: - dvc_file_path = stage_file - else: - dvc_file_name = self.DEF_DVC_FILE_PREFIX + module_name + ".dvc" - dvc_file_path = os.path.join(target_dir, dvc_file_name) - return dvc_file_path - - -def install(self, address, target_dir, select=[], fname=None): - """ - Install package. - - The command can be run only from DVC project root. - - E.g. - Having: DVC package in https://github.com/dmpetrov/tag_classifier - - $ dvc pkg install https://github.com/dmpetrov/tag_classifier - - Result: tag_classifier package in dvc_mod/ directory - """ - - if not os.path.isdir(target_dir): - raise DvcException( - "target directory '{}' does not exist".format(target_dir) - ) - - curr_dir = os.path.realpath(os.curdir) - if not os.path.realpath(target_dir).startswith(curr_dir): - raise DvcException( - "the current directory should be a subdirectory of the target " - "dir '{}'".format(target_dir) - ) - - addresses = [address] if address else PackageManager.read_packages() - for addr in addresses: - mgr = PackageManager.get_package(addr) - mgr.install_or_update(self, address, target_dir, select, fname) From 91bd0faa87560edb5df97371a1af8e1025008d32 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 26 Jun 2019 16:25:56 -0500 Subject: [PATCH 06/13] dvc: feedback to #2112 round 1 --- dvc/stage.py | 4 ++-- tests/func/test_add.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dvc/stage.py b/dvc/stage.py index 285a12336..4b76b0e2b 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -206,7 +206,7 @@ class Stage(object): @property def is_callback(self): """ - An orphan stage is always considered as changed, + A callback stage is always considered as changed, so it runs on every `dvc repro` call. """ return not self.is_data_source and len(self.deps) == 0 @@ -229,7 +229,7 @@ class Stage(object): if self.is_callback: logger.warning( - "DVC-file '{fname}' is an 'orphan' stage " + "DVC-file '{fname}' is an 'callback' stage " "(has a command and no dependencies) and thus always " "considered as changed.".format(fname=self.relpath) ) diff --git a/tests/func/test_add.py b/tests/func/test_add.py index ce3a8fecd..2c1aa2927 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -424,7 +424,7 @@ class TestShouldThrowProperExceptionOnCorruptedStageFile(TestDvc): foo_stage = relpath(self.FOO + Stage.STAGE_FILE_SUFFIX) - # corrupt DVC-file + # corrupt stage file with open(foo_stage, "a+") as file: file.write("this will break yaml file structure") @@ -475,7 +475,8 @@ class TestShouldCleanUpAfterFailedAdd(TestDvcGit): self.assertEqual(0, ret) foo_stage_file = self.FOO + Stage.STAGE_FILE_SUFFIX - # corrupt DVC-file + + # corrupt stage file with open(foo_stage_file, "a+") as file: file.write("this will break yaml file structure") From e3f1e61c28dc0d136c5222d574dfb35b49fefb25 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 26 Jun 2019 21:07:09 -0500 Subject: [PATCH 07/13] data sync: small improvement continuing #2121 and matching https://github.com/jorgeorpinel/dvc.org/commit/dd13134 --- dvc/command/data_sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 6295f4c4d..7945435a2 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -118,8 +118,8 @@ def shared_parent_parser(): nargs="*", default=None, help=( - "Limit the scope to these DVC-files." - " With -R a directory to search DVC-files in can be specified." + "Limit command scope to these DVC-files." + " Using -R, directories to search DVC-files in can also be given." ), ) From 936f941bf060979c40ebe80e955580223de93079 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 26 Jun 2019 23:25:58 -0500 Subject: [PATCH 08/13] help: update usage for commands that use the `targets` argument --- dvc/command/add.py | 2 +- dvc/command/checkout.py | 7 ++++++- dvc/command/commit.py | 6 +++++- dvc/command/data_sync.py | 6 ++---- dvc/command/lock.py | 12 ++++++++---- dvc/command/pipeline.py | 7 +++++-- dvc/command/remove.py | 9 +++++++-- dvc/command/repro.py | 4 ++-- dvc/command/unprotect.py | 4 ++-- 9 files changed, 38 insertions(+), 19 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 45c74c06f..16b3b979e 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -57,6 +57,6 @@ def add_parser(subparsers, parent_parser): "-f", "--file", help="Specify name of the DVC-file it generates." ) add_parser.add_argument( - "targets", nargs="+", help="Input files/directories." + "targets", nargs="+", help="Input files/directories to add." ) add_parser.set_defaults(func=CmdAdd) diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index 6637140b2..cf3560f39 100644 --- a/dvc/command/checkout.py +++ b/dvc/command/checkout.py @@ -51,5 +51,10 @@ def add_parser(subparsers, parent_parser): default=False, help="Checkout all subdirectories of the specified directory.", ) - checkout_parser.add_argument("targets", nargs="*", help="DVC-files.") + checkout_parser.add_argument( + "targets", + nargs="*", + help="DVC-files to checkout. Optional." + "(Finds all DVC-files in the workspace by default.)", + ) checkout_parser.set_defaults(func=CmdCheckout) diff --git a/dvc/command/commit.py b/dvc/command/commit.py index 8ba3b99f3..ef703e117 100644 --- a/dvc/command/commit.py +++ b/dvc/command/commit.py @@ -65,6 +65,10 @@ def add_parser(subparsers, parent_parser): help="Commit cache for subdirectories of the specified directory.", ) commit_parser.add_argument( - "targets", nargs="*", default=None, help="DVC-files." + "targets", + nargs="*", + default=None, + help="DVC-files to commit. Optional." + "(Finds all DVC-files in the workspace by default.)", ) commit_parser.set_defaults(func=CmdCommit) diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 7945435a2..b8319b8d4 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -117,10 +117,8 @@ def shared_parent_parser(): "targets", nargs="*", default=None, - help=( - "Limit command scope to these DVC-files." - " Using -R, directories to search DVC-files in can also be given." - ), + help="Limit command scope to these DVC-files. " + "Using -R, directories to search DVC-files in can also be given.", ) return shared_parent_parser diff --git a/dvc/command/lock.py b/dvc/command/lock.py index e934380ed..b231783e7 100644 --- a/dvc/command/lock.py +++ b/dvc/command/lock.py @@ -37,7 +37,7 @@ class CmdUnlock(CmdLockBase): def add_parser(subparsers, parent_parser): - LOCK_HELP = "Lock DVC-file." + LOCK_HELP = "Lock DVC-files." lock_parser = subparsers.add_parser( "lock", parents=[parent_parser], @@ -45,10 +45,12 @@ def add_parser(subparsers, parent_parser): help=LOCK_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - lock_parser.add_argument("targets", nargs="+", help="DVC-files.") + lock_parser.add_argument( + "targets", nargs="*", help="DVC-files to lock. Optional." + ) lock_parser.set_defaults(func=CmdLock) - UNLOCK_HELP = "Unlock DVC-file." + UNLOCK_HELP = "Unlock DVC-files." unlock_parser = subparsers.add_parser( "unlock", parents=[parent_parser], @@ -56,5 +58,7 @@ def add_parser(subparsers, parent_parser): help=UNLOCK_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - unlock_parser.add_argument("targets", nargs="+", help="DVC-files.") + unlock_parser.add_argument( + "targets", nargs="*", help="DVC-files to unlock. Optional." + ) unlock_parser.set_defaults(func=CmdUnlock) diff --git a/dvc/command/pipeline.py b/dvc/command/pipeline.py index 70b8bd79d..d578b2493 100644 --- a/dvc/command/pipeline.py +++ b/dvc/command/pipeline.py @@ -197,7 +197,7 @@ def add_parser(subparsers, parent_parser): fix_subparsers(pipeline_subparsers) - PIPELINE_SHOW_HELP = "Show pipeline." + PIPELINE_SHOW_HELP = "Show pipelines." pipeline_show_parser = pipeline_subparsers.add_parser( "show", parents=[parent_parser], @@ -246,7 +246,10 @@ def add_parser(subparsers, parent_parser): help="Output DAG as Dependencies Tree.", ) pipeline_show_parser.add_argument( - "targets", nargs="*", help="DVC-files. 'Dvcfile' by default." + "targets", + nargs="*", + help="DVC-files to show pipeline for. Optional." + "(Finds all DVC-files in the workspace by default.)", ) pipeline_show_parser.set_defaults(func=CmdPipelineShow) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 4a006a575..0a308f0c5 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -43,7 +43,7 @@ class CmdRemove(CmdBase): def add_parser(subparsers, parent_parser): - REMOVE_HELP = "Remove outputs of DVC-file." + REMOVE_HELP = "Remove DVC-file outputs." remove_parser = subparsers.add_parser( "remove", parents=[parent_parser], @@ -73,5 +73,10 @@ def add_parser(subparsers, parent_parser): default=False, help="Force purge.", ) - remove_parser.add_argument("targets", nargs="+", help="DVC-files.") + remove_parser.add_argument( + "targets", + nargs="*", + help="DVC-files to remove. Optional." + "(Finds all DVC-files in the workspace by default.)", + ) remove_parser.set_defaults(func=CmdRemove) diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 3c7ee2f21..707657139 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -58,7 +58,7 @@ class CmdRepro(CmdBase): def add_parser(subparsers, parent_parser): - REPRO_HELP = "Check for changes and reproduce DVC-file and dependencies." + REPRO_HELP = "Check for changes and reproduce stages and dependencies." repro_parser = subparsers.add_parser( "repro", parents=[parent_parser], @@ -69,7 +69,7 @@ def add_parser(subparsers, parent_parser): repro_parser.add_argument( "targets", nargs="*", - help="DVC-file to reproduce (default - 'Dvcfile').", + help="DVC-file to reproduce. 'Dvcfile' by default.", ) repro_parser.add_argument( "-f", diff --git a/dvc/command/unprotect.py b/dvc/command/unprotect.py index e62848992..b4b276234 100644 --- a/dvc/command/unprotect.py +++ b/dvc/command/unprotect.py @@ -23,7 +23,7 @@ class CmdUnprotect(CmdBase): def add_parser(subparsers, parent_parser): - UNPROTECT_HELP = "Unprotect a data file/directory." + UNPROTECT_HELP = "Unprotect data files or directories." unprotect_parser = subparsers.add_parser( "unprotect", parents=[parent_parser], @@ -32,6 +32,6 @@ def add_parser(subparsers, parent_parser): formatter_class=argparse.RawDescriptionHelpFormatter, ) unprotect_parser.add_argument( - "targets", nargs="+", help="Data files/directory." + "targets", nargs="+", help="Data files/directories to unprotect." ) unprotect_parser.set_defaults(func=CmdUnprotect) From a927a506a75d6d2991324ad2b6f371ce9844f7da Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 27 Jun 2019 00:22:10 -0500 Subject: [PATCH 09/13] command: remove `default` property from "targets" argparse code in data sync and `commit` commands, since `None` is the default value for the removed `default` property, see https://docs.python.org/dev/library/argparse.html#default --- dvc/command/commit.py | 1 - dvc/command/data_sync.py | 1 - 2 files changed, 2 deletions(-) diff --git a/dvc/command/commit.py b/dvc/command/commit.py index ef703e117..731b44bac 100644 --- a/dvc/command/commit.py +++ b/dvc/command/commit.py @@ -67,7 +67,6 @@ def add_parser(subparsers, parent_parser): commit_parser.add_argument( "targets", nargs="*", - default=None, help="DVC-files to commit. Optional." "(Finds all DVC-files in the workspace by default.)", ) diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index b8319b8d4..17bebcb38 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -116,7 +116,6 @@ def shared_parent_parser(): shared_parent_parser.add_argument( "targets", nargs="*", - default=None, help="Limit command scope to these DVC-files. " "Using -R, directories to search DVC-files in can also be given.", ) From 5ba3b547ba1f6e235fe06a0f55f763ef1790fc1a Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 27 Jun 2019 12:23:45 -0500 Subject: [PATCH 10/13] command: rever accidental `narg="+"` changes in recent commit Per https://github.com/iterative/dvc/pull/2112#pullrequestreview-255082022 --- dvc/command/lock.py | 4 ++-- dvc/command/remove.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/command/lock.py b/dvc/command/lock.py index b231783e7..a36b3cb76 100644 --- a/dvc/command/lock.py +++ b/dvc/command/lock.py @@ -46,7 +46,7 @@ def add_parser(subparsers, parent_parser): formatter_class=argparse.RawDescriptionHelpFormatter, ) lock_parser.add_argument( - "targets", nargs="*", help="DVC-files to lock. Optional." + "targets", nargs="+", help="DVC-files to lock. Optional." ) lock_parser.set_defaults(func=CmdLock) @@ -59,6 +59,6 @@ def add_parser(subparsers, parent_parser): formatter_class=argparse.RawDescriptionHelpFormatter, ) unlock_parser.add_argument( - "targets", nargs="*", help="DVC-files to unlock. Optional." + "targets", nargs="+", help="DVC-files to unlock. Optional." ) unlock_parser.set_defaults(func=CmdUnlock) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 0a308f0c5..130efadbf 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -75,7 +75,7 @@ def add_parser(subparsers, parent_parser): ) remove_parser.add_argument( "targets", - nargs="*", + nargs="+", help="DVC-files to remove. Optional." "(Finds all DVC-files in the workspace by default.)", ) From d0718cd91e8f4ca332697bd2cc58b85815a8caec Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 27 Jun 2019 12:29:17 -0500 Subject: [PATCH 11/13] stage: fix typo "an"->"a" Per https://github.com/iterative/dvc/pull/2112#pullrequestreview-255087237 --- dvc/stage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/stage.py b/dvc/stage.py index 4b76b0e2b..c88d5b978 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -229,7 +229,7 @@ class Stage(object): if self.is_callback: logger.warning( - "DVC-file '{fname}' is an 'callback' stage " + "DVC-file '{fname}' is a 'callback' stage " "(has a command and no dependencies) and thus always " "considered as changed.".format(fname=self.relpath) ) From a2f3d7fcfaacef50155d0b15a67525d439563e44 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 27 Jun 2019 13:39:05 -0500 Subject: [PATCH 12/13] command: add missing space in optional `targets` ard usage output --- dvc/command/checkout.py | 2 +- dvc/command/commit.py | 2 +- dvc/command/pipeline.py | 2 +- dvc/command/remove.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index cf3560f39..5d1941587 100644 --- a/dvc/command/checkout.py +++ b/dvc/command/checkout.py @@ -54,7 +54,7 @@ def add_parser(subparsers, parent_parser): checkout_parser.add_argument( "targets", nargs="*", - help="DVC-files to checkout. Optional." + help="DVC-files to checkout. Optional. " "(Finds all DVC-files in the workspace by default.)", ) checkout_parser.set_defaults(func=CmdCheckout) diff --git a/dvc/command/commit.py b/dvc/command/commit.py index 731b44bac..daa4d2474 100644 --- a/dvc/command/commit.py +++ b/dvc/command/commit.py @@ -67,7 +67,7 @@ def add_parser(subparsers, parent_parser): commit_parser.add_argument( "targets", nargs="*", - help="DVC-files to commit. Optional." + help="DVC-files to commit. Optional. " "(Finds all DVC-files in the workspace by default.)", ) commit_parser.set_defaults(func=CmdCommit) diff --git a/dvc/command/pipeline.py b/dvc/command/pipeline.py index d578b2493..8b7bc2f40 100644 --- a/dvc/command/pipeline.py +++ b/dvc/command/pipeline.py @@ -248,7 +248,7 @@ def add_parser(subparsers, parent_parser): pipeline_show_parser.add_argument( "targets", nargs="*", - help="DVC-files to show pipeline for. Optional." + help="DVC-files to show pipeline for. Optional. " "(Finds all DVC-files in the workspace by default.)", ) pipeline_show_parser.set_defaults(func=CmdPipelineShow) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 130efadbf..b8e98200c 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -76,7 +76,7 @@ def add_parser(subparsers, parent_parser): remove_parser.add_argument( "targets", nargs="+", - help="DVC-files to remove. Optional." + help="DVC-files to remove. Optional. " "(Finds all DVC-files in the workspace by default.)", ) remove_parser.set_defaults(func=CmdRemove) From 5bf6eaac1b5aa1911922cd44ab3a52a683c50f89 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 27 Jun 2019 13:44:05 -0500 Subject: [PATCH 13/13] lock: lock/unlock targets are NOT optional. Continuation of fix in 5ba3b54 --- dvc/command/lock.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dvc/command/lock.py b/dvc/command/lock.py index a36b3cb76..da672c352 100644 --- a/dvc/command/lock.py +++ b/dvc/command/lock.py @@ -45,9 +45,7 @@ def add_parser(subparsers, parent_parser): help=LOCK_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) - lock_parser.add_argument( - "targets", nargs="+", help="DVC-files to lock. Optional." - ) + lock_parser.add_argument("targets", nargs="+", help="DVC-files to lock.") lock_parser.set_defaults(func=CmdLock) UNLOCK_HELP = "Unlock DVC-files." @@ -59,6 +57,6 @@ def add_parser(subparsers, parent_parser): formatter_class=argparse.RawDescriptionHelpFormatter, ) unlock_parser.add_argument( - "targets", nargs="+", help="DVC-files to unlock. Optional." + "targets", nargs="+", help="DVC-files to unlock." ) unlock_parser.set_defaults(func=CmdUnlock)