From 970481039896a8ec4cfdd2e9115cb59b3b9a52b1 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 28 Sep 2019 16:56:14 +0100 Subject: [PATCH 01/22] some doc improvements --- dvc/command/add.py | 15 +++++++++------ dvc/command/base.py | 4 ++-- dvc/exceptions.py | 4 ++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 16b3b979e9..09c3d9b7fb 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -5,6 +5,7 @@ from dvc.exceptions import DvcException from dvc.command.base import CmdBase, append_doc_link +from dvc.exceptions import RecursiveAddingWhileUsingFilename logger = logging.getLogger(__name__) @@ -14,7 +15,9 @@ class CmdAdd(CmdBase): def run(self): try: if len(self.args.targets) > 1 and self.args.file: - raise DvcException("can't use '--file' with multiple targets") + raise RecursiveAddingWhileUsingFilename( + "can't use '--file' with multiple targets" + ) for target in self.args.targets: self.repo.add( @@ -24,14 +27,14 @@ def run(self): fname=self.args.file, ) - except DvcException: - logger.exception("failed to add file") + except DvcException as e: + logger.exception("{}:{}".format(type(e).__name__, e)) return 1 return 0 def add_parser(subparsers, parent_parser): - ADD_HELP = "Take data files or directories under DVC control." + ADD_HELP = "Add data files or directories to DVC control." add_parser = subparsers.add_parser( "add", @@ -45,7 +48,7 @@ def add_parser(subparsers, parent_parser): "--recursive", action="store_true", default=False, - help="Recursively add each file under the directory.", + help="Recursively add files under directories.", ) add_parser.add_argument( "--no-commit", @@ -54,7 +57,7 @@ def add_parser(subparsers, parent_parser): help="Don't put files/directories into cache.", ) add_parser.add_argument( - "-f", "--file", help="Specify name of the DVC-file it generates." + "-f", "--file", help="Specify name of the generated DVC file." ) add_parser.add_argument( "targets", nargs="+", help="Input files/directories to add." diff --git a/dvc/command/base.py b/dvc/command/base.py index f4d44b2425..e17ba06f38 100644 --- a/dvc/command/base.py +++ b/dvc/command/base.py @@ -26,7 +26,7 @@ def append_doc_link(help_message, path): if not path: return help_message doc_base = "https://man.dvc.org/" - return "{message}\ndocumentation: {blue}{base}{path}{nc}".format( + return "{message}\nDocumentation: <{blue}{base}{path}{nc}>".format( message=help_message, base=doc_base, path=path, @@ -57,7 +57,7 @@ def default_targets(self): # Abstract methods that have to be implemented by any inheritance class def run(self): - pass + raise NotImplementedError("Abstract") class CmdBaseNoRepo(CmdBase): diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 111f26b928..04a12b69d0 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -224,9 +224,9 @@ def __init__(self, path, cause=None): class RecursiveAddingWhileUsingFilename(DvcException): - def __init__(self): + def __init__(self, msg=None): super(RecursiveAddingWhileUsingFilename, self).__init__( - "using fname with recursive is not allowed." + msg or "using fname with recursive is not allowed." ) From f2db8d0eeba84475d171bf860a9d4f97974f59b4 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 28 Sep 2019 16:56:33 +0100 Subject: [PATCH 02/22] add: intermediate progress --- dvc/command/add.py | 19 ++++++++++++------- dvc/repo/add.py | 16 +++++++++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 09c3d9b7fb..eaf32b0d74 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -6,6 +6,7 @@ from dvc.exceptions import DvcException from dvc.command.base import CmdBase, append_doc_link from dvc.exceptions import RecursiveAddingWhileUsingFilename +from dvc.progress import Tqdm logger = logging.getLogger(__name__) @@ -19,13 +20,17 @@ def run(self): "can't use '--file' with multiple targets" ) - for target in self.args.targets: - self.repo.add( - target, - recursive=self.args.recursive, - no_commit=self.args.no_commit, - fname=self.args.file, - ) + with Tqdm( + total=len(self.args.targets), desc="Adding", unit="file" + ) as pbar: + for target in self.args.targets: + stages = self.repo.add( + target, + recursive=self.args.recursive, + no_commit=self.args.no_commit, + fname=self.args.file, + pbar=pbar, + ) except DvcException as e: logger.exception("{}:{}".format(type(e).__name__, e)) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index ce2b4d1d09..98f76a6065 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -17,11 +17,13 @@ @locked @scm_context -def add(repo, target, recursive=False, no_commit=False, fname=None): +def add(repo, target, recursive=False, no_commit=False, fname=None, pbar=None): if recursive and fname: raise RecursiveAddingWhileUsingFilename() targets = _find_all_targets(repo, target, recursive) + if pbar is not None: + pbar.total += len(targets) - 1 if os.path.isdir(target) and len(targets) > LARGE_DIR_SIZE: logger.warning( @@ -36,7 +38,12 @@ def add(repo, target, recursive=False, no_commit=False, fname=None): ) ) - stages = _create_stages(repo, targets, fname) + stages = _create_stages( + repo, + targets, + fname, + callback=pbar.update_desc if pbar is not None else None + ) repo.check_modified_graph(stages) @@ -64,7 +71,7 @@ def _find_all_targets(repo, target, recursive): return [target] -def _create_stages(repo, targets, fname): +def _create_stages(repo, targets, fname, callback=None): stages = [] for out in targets: @@ -75,4 +82,7 @@ def _create_stages(repo, targets, fname): stages.append(stage) + if callback is not None: + callback(out) + return stages From 1909050c927508f9098174a2a45b4f51b4e517bd Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 28 Sep 2019 22:51:55 +0100 Subject: [PATCH 03/22] update after rebase --- dvc/command/add.py | 2 +- dvc/repo/add.py | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index eaf32b0d74..d1cd6f0c52 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -24,7 +24,7 @@ def run(self): total=len(self.args.targets), desc="Adding", unit="file" ) as pbar: for target in self.args.targets: - stages = self.repo.add( + self.repo.add( target, recursive=self.args.recursive, no_commit=self.args.no_commit, diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 98f76a6065..d3ede7f2cd 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -38,12 +38,7 @@ def add(repo, target, recursive=False, no_commit=False, fname=None, pbar=None): ) ) - stages = _create_stages( - repo, - targets, - fname, - callback=pbar.update_desc if pbar is not None else None - ) + stages = _create_stages(repo, targets, fname, pbar=pbar) repo.check_modified_graph(stages) @@ -71,18 +66,19 @@ def _find_all_targets(repo, target, recursive): return [target] -def _create_stages(repo, targets, fname, callback=None): +def _create_stages(repo, targets, fname, pbar=None): stages = [] for out in targets: stage = Stage.create(repo, outs=[out], add=True, fname=fname) if not stage: + if pbar is not None: + pbar.total -= 1 continue stages.append(stage) - - if callback is not None: - callback(out) + if pbar is not None: + pbar.update_desc(out) return stages From 4aebdf728d128e72f3d13cf218faba5d9aaea8f6 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 28 Sep 2019 23:35:40 +0100 Subject: [PATCH 04/22] linting --- dvc/command/add.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index d1cd6f0c52..de58130689 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -32,20 +32,20 @@ def run(self): pbar=pbar, ) - except DvcException as e: - logger.exception("{}:{}".format(type(e).__name__, e)) + except DvcException as err: + logger.exception("{}:{}".format(type(err).__name__, err)) return 1 return 0 def add_parser(subparsers, parent_parser): - ADD_HELP = "Add data files or directories to DVC control." + add_help = "Add data files or directories to DVC control." add_parser = subparsers.add_parser( "add", parents=[parent_parser], - description=append_doc_link(ADD_HELP, "add"), - help=ADD_HELP, + description=append_doc_link(add_help, "add"), + help=add_help, formatter_class=argparse.RawDescriptionHelpFormatter, ) add_parser.add_argument( From c941c015a9b70eeede302dd3cf43c7ce21a4ecc8 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 12 Oct 2019 21:16:23 +0100 Subject: [PATCH 05/22] fix checksum progress cleanup by ensuring gc --- dvc/remote/base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 5dd2ca1b6a..eb7eb1e0aa 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -187,6 +187,7 @@ def _calculate_checksums(self, file_infos): ) tasks = Tqdm(tasks, total=len(file_infos), unit="md5") checksums = dict(zip(file_infos, tasks)) + del tasks return checksums def _collect_dir(self, path_info): From 06b4495dbc9c06114a0ea99d4bb817d85e35654f Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 12 Oct 2019 21:47:13 +0100 Subject: [PATCH 06/22] progress-aware logger.info --- dvc/logger.py | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/dvc/logger.py b/dvc/logger.py index 9fab58634e..95ea0a1814 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -22,6 +22,11 @@ def filter(self, record): return record.levelno < logging.WARNING +class ExcludeInfoFilter(logging.Filter): + def filter(self, record): + return record.levelno < logging.INFO + + class ColorFormatter(logging.Formatter): """Enable color support when logging to a terminal that supports it. @@ -170,15 +175,25 @@ def setup(level=logging.INFO): logging.config.dictConfig( { "version": 1, - "filters": {"exclude_errors": {"()": ExcludeErrorsFilter}}, + "filters": { + "exclude_errors": {"()": ExcludeErrorsFilter}, + "exclude_info": {"()": ExcludeInfoFilter}, + }, "formatters": {"color": {"()": ColorFormatter}}, "handlers": { - "console": { + "console_info": { + "class": "dvc.logger.LoggerHandler", + "level": "INFO", + # "formatter": "color", + "stream": "ext://sys.stdout", + "filters": ["exclude_errors"], + }, + "console_debug": { "class": "dvc.logger.LoggerHandler", "level": "DEBUG", "formatter": "color", "stream": "ext://sys.stdout", - "filters": ["exclude_errors"], + "filters": ["exclude_info"], }, "console_errors": { "class": "dvc.logger.LoggerHandler", @@ -190,15 +205,27 @@ def setup(level=logging.INFO): "loggers": { "dvc": { "level": level, - "handlers": ["console", "console_errors"], + "handlers": [ + "console_info", + "console_debug", + "console_errors", + ], }, "paramiko": { "level": logging.CRITICAL, - "handlers": ["console", "console_errors"], + "handlers": [ + "console_info", + "console_debug", + "console_errors", + ], }, "flufl.lock": { "level": logging.CRITICAL, - "handlers": ["console", "console_errors"], + "handlers": [ + "console_info", + "console_debug", + "console_errors", + ], }, }, } From 6ac8935d5302735b1339d77650d95f72d73b54e2 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 12 Oct 2019 21:47:27 +0100 Subject: [PATCH 07/22] re-fix checksum pbar closure --- dvc/remote/base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dvc/remote/base.py b/dvc/remote/base.py index eb7eb1e0aa..4b70301ffe 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -187,7 +187,8 @@ def _calculate_checksums(self, file_infos): ) tasks = Tqdm(tasks, total=len(file_infos), unit="md5") checksums = dict(zip(file_infos, tasks)) - del tasks + if hasattr(tasks, "close"): + tasks.close() return checksums def _collect_dir(self, path_info): From 0913ea659ed4011d5650d61b342948889fe2b68d Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 14 Oct 2019 16:18:10 +0100 Subject: [PATCH 08/22] multi-file add --- dvc/command/add.py | 19 +++++-------- dvc/repo/add.py | 71 +++++++++++++++++++++++++++++----------------- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index de58130689..22b9228503 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -6,7 +6,6 @@ from dvc.exceptions import DvcException from dvc.command.base import CmdBase, append_doc_link from dvc.exceptions import RecursiveAddingWhileUsingFilename -from dvc.progress import Tqdm logger = logging.getLogger(__name__) @@ -20,17 +19,13 @@ def run(self): "can't use '--file' with multiple targets" ) - with Tqdm( - total=len(self.args.targets), desc="Adding", unit="file" - ) as pbar: - for target in self.args.targets: - self.repo.add( - target, - recursive=self.args.recursive, - no_commit=self.args.no_commit, - fname=self.args.file, - pbar=pbar, - ) + self.repo.add( + self.args.targets, + recursive=self.args.recursive, + no_commit=self.args.no_commit, + fname=self.args.file, + progress=True, + ) except DvcException as err: logger.exception("{}:{}".format(type(err).__name__, err)) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index d3ede7f2cd..461fc51711 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -3,54 +3,73 @@ import os import logging import colorama +from six import string_types from dvc.repo.scm_context import scm_context from dvc.stage import Stage from dvc.utils import walk_files, LARGE_DIR_SIZE from dvc.exceptions import RecursiveAddingWhileUsingFilename - +from dvc.progress import Tqdm from . import locked - logger = logging.getLogger(__name__) @locked @scm_context -def add(repo, target, recursive=False, no_commit=False, fname=None, pbar=None): +def add( + repo, + target_list, + recursive=False, + no_commit=False, + fname=None, + progress=False, +): if recursive and fname: raise RecursiveAddingWhileUsingFilename() - targets = _find_all_targets(repo, target, recursive) - if pbar is not None: - pbar.total += len(targets) - 1 + if isinstance(target_list, string_types): + target_list = [target_list] - if os.path.isdir(target) and len(targets) > LARGE_DIR_SIZE: - logger.warning( - "You are adding a large directory '{target}' recursively," - " consider tracking it as a whole instead.\n" - "{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, - nc=colorama.Style.RESET_ALL, - target=target, - ) - ) + stages_list = [] + with Tqdm( + total=len(target_list), + desc="Adding", + unit="file", + disable=not progress, + ) as pbar: + for target in target_list: + targets = _find_all_targets(repo, target, recursive) + pbar.total += len(targets) - 1 - stages = _create_stages(repo, targets, fname, pbar=pbar) + if os.path.isdir(target) and len(targets) > LARGE_DIR_SIZE: + logger.warning( + "You are adding a large directory '{target}' recursively," + " consider tracking it as a whole instead.\n" + "{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, + nc=colorama.Style.RESET_ALL, + target=target, + ) + ) - repo.check_modified_graph(stages) + stages = _create_stages(repo, targets, fname, pbar=pbar) - for stage in stages: - stage.save() + repo.check_modified_graph(stages) - if not no_commit: - stage.commit() + for stage in stages: + stage.save() - stage.dump() + if not no_commit: + stage.commit() - return stages + stage.dump() + + stages_list += stages + + return stages_list def _find_all_targets(repo, target, recursive): From f6fb05a1755a3519671b70d478bea42742f57b33 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 17 Oct 2019 20:39:54 +0100 Subject: [PATCH 09/22] make stage.dump() less verbose. TODO: check all instances of stage.dump and possibly make verbose --- dvc/stage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/stage.py b/dvc/stage.py index a368788486..4d3b7d3d0e 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -663,7 +663,7 @@ def dump(self): self._check_dvc_filename(fname) - logger.info( + logger.debug( "Saving information to '{file}'.".format(file=relpath(fname)) ) d = self.dumpd() From 1c1763e829705e31b10ac8f5050afc1add77c7cb Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 17 Oct 2019 20:47:00 +0100 Subject: [PATCH 10/22] decrease verbosity of checksum calculations --- dvc/remote/base.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 4b70301ffe..470b7d6e57 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -22,7 +22,7 @@ DvcIgnoreInCollectedDirError, ) from dvc.progress import Tqdm -from dvc.utils import LARGE_DIR_SIZE, tmp_fname, move, relpath, makedirs +from dvc.utils import tmp_fname, move, relpath, makedirs from dvc.state import StateNoop from dvc.path_info import PathInfo, URLInfo from dvc.utils.http import open_url @@ -177,18 +177,13 @@ def _calculate_checksums(self, file_infos): file_infos = list(file_infos) with ThreadPoolExecutor(max_workers=self.checksum_jobs) as executor: tasks = executor.map(self.get_file_checksum, file_infos) - - if len(file_infos) > LARGE_DIR_SIZE: - logger.info( - ( - "Computing md5 for a large number of files. " - "This is only done once." - ) - ) - tasks = Tqdm(tasks, total=len(file_infos), unit="md5") - checksums = dict(zip(file_infos, tasks)) - if hasattr(tasks, "close"): - tasks.close() + with Tqdm( + tasks, + total=len(file_infos), + unit="md5", + desc="Computing hashes (only done once)", + ) as tasks: + checksums = dict(zip(file_infos, tasks)) return checksums def _collect_dir(self, path_info): From bf0a32294ddca0489e7917954d702a8acf302c83 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 17 Oct 2019 21:18:18 +0100 Subject: [PATCH 11/22] leave dvc.add stats --- dvc/repo/add.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 461fc51711..c6970a21f9 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -37,6 +37,7 @@ def add( desc="Adding", unit="file", disable=not progress, + leave=True, ) as pbar: for target in target_list: targets = _find_all_targets(repo, target, recursive) @@ -68,6 +69,7 @@ def add( stage.dump() stages_list += stages + pbar.bar_format = pbar.BAR_FMT_DEFAULT.replace("|{bar:10}|", " ") return stages_list From bdc8c70018ca51a0998a50b22af693b46c0ab511 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 17 Oct 2019 21:30:34 +0100 Subject: [PATCH 12/22] update/fix logger tests --- tests/unit/test_logger.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 3560018a9d..5560acd4ea 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -182,7 +182,8 @@ def test_progress_awareness(self, mocker, capsys, caplog): def test_handlers(): - stdout, stderr = logger.handlers + out, deb, err = logger.handlers - assert stdout.level == logging.DEBUG - assert stderr.level == logging.WARNING + assert out.level == logging.INFO + assert deb.level == logging.DEBUG + assert err.level == logging.WARNING From 17383ae39b4490ed0dfd4f791734f13028623fcc Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 21 Oct 2019 14:56:28 +0100 Subject: [PATCH 13/22] minor doc reword Co-Authored-By: Ruslan Kuprieiev --- dvc/command/add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 22b9228503..1bcf45701b 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -57,7 +57,7 @@ def add_parser(subparsers, parent_parser): help="Don't put files/directories into cache.", ) add_parser.add_argument( - "-f", "--file", help="Specify name of the generated DVC file." + "-f", "--file", help="Specify name of the generated DVC-file." ) add_parser.add_argument( "targets", nargs="+", help="Input files/directories to add." From be7e7667fde85af9e5235741ba632d5b5551d4b7 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 21 Oct 2019 15:19:08 +0100 Subject: [PATCH 14/22] tidy API target_;list => targets - as per https://github.com/iterative/dvc/pull/2546#discussion_r337042535 --- dvc/repo/add.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index c6970a21f9..f7381e853a 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -18,32 +18,27 @@ @locked @scm_context def add( - repo, - target_list, - recursive=False, - no_commit=False, - fname=None, - progress=False, + repo, targets, recursive=False, no_commit=False, fname=None, progress=False ): if recursive and fname: raise RecursiveAddingWhileUsingFilename() - if isinstance(target_list, string_types): - target_list = [target_list] + if isinstance(targets, string_types): + targets = [targets] stages_list = [] with Tqdm( - total=len(target_list), + total=len(targets), desc="Adding", unit="file", disable=not progress, leave=True, ) as pbar: - for target in target_list: - targets = _find_all_targets(repo, target, recursive) - pbar.total += len(targets) - 1 + for target in targets: + sub_targets = _find_all_targets(repo, target, recursive) + pbar.total += len(sub_targets) - 1 - if os.path.isdir(target) and len(targets) > LARGE_DIR_SIZE: + if os.path.isdir(target) and len(sub_targets) > LARGE_DIR_SIZE: logger.warning( "You are adding a large directory '{target}' recursively," " consider tracking it as a whole instead.\n" @@ -56,7 +51,7 @@ def add( ) ) - stages = _create_stages(repo, targets, fname, pbar=pbar) + stages = _create_stages(repo, sub_targets, fname, pbar=pbar) repo.check_modified_graph(stages) From ccf45ea0677a6b7fd62f76512afdf22b3d9005d3 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 21 Oct 2019 15:27:30 +0100 Subject: [PATCH 15/22] tidy RecursiveAddingWhileUsingFilename message duplication - fixes https://github.com/iterative/dvc/pull/2546#discussion_r337044670 --- dvc/command/add.py | 4 +--- dvc/exceptions.py | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 1bcf45701b..9d3a342351 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -15,9 +15,7 @@ class CmdAdd(CmdBase): def run(self): try: if len(self.args.targets) > 1 and self.args.file: - raise RecursiveAddingWhileUsingFilename( - "can't use '--file' with multiple targets" - ) + raise RecursiveAddingWhileUsingFilename() self.repo.add( self.args.targets, diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 04a12b69d0..2309bcdbf8 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -224,9 +224,9 @@ def __init__(self, path, cause=None): class RecursiveAddingWhileUsingFilename(DvcException): - def __init__(self, msg=None): + def __init__(self): super(RecursiveAddingWhileUsingFilename, self).__init__( - msg or "using fname with recursive is not allowed." + "cannot use `fname` with multiple targets or `-R|--recursive`" ) From 1bda5e2159e73d9861a23f63c10253dc4a258b80 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 21 Oct 2019 15:31:06 +0100 Subject: [PATCH 16/22] minor logging uncomment - fixes https://github.com/iterative/dvc/pull/2546#discussion_r337051097 --- dvc/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/logger.py b/dvc/logger.py index 95ea0a1814..71f9d9147c 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -184,7 +184,7 @@ def setup(level=logging.INFO): "console_info": { "class": "dvc.logger.LoggerHandler", "level": "INFO", - # "formatter": "color", + "formatter": "color", "stream": "ext://sys.stdout", "filters": ["exclude_errors"], }, From 9125ae719abd84e179eec6321314098c4860fd1f Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 21 Oct 2019 18:18:14 +0100 Subject: [PATCH 17/22] minor doc improvements - https://github.com/iterative/dvc/pull/2546#discussion_r337116255 - https://github.com/iterative/dvc/pull/2546#discussion_r337116809 --- dvc/command/add.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 9d3a342351..170bc13171 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -34,30 +34,32 @@ def run(self): def add_parser(subparsers, parent_parser): add_help = "Add data files or directories to DVC control." - add_parser = subparsers.add_parser( + parser = subparsers.add_parser( "add", parents=[parent_parser], description=append_doc_link(add_help, "add"), help=add_help, formatter_class=argparse.RawDescriptionHelpFormatter, ) - add_parser.add_argument( + parser.add_argument( "-R", "--recursive", action="store_true", default=False, - help="Recursively add files under directories.", + help="Recursively add files under directory targets.", ) - add_parser.add_argument( + parser.add_argument( "--no-commit", action="store_true", default=False, help="Don't put files/directories into cache.", ) - add_parser.add_argument( - "-f", "--file", help="Specify name of the generated DVC-file." + parser.add_argument( + "-f", + "--file", + help="Specify name of the DVC-file this command will generate.", ) - add_parser.add_argument( + parser.add_argument( "targets", nargs="+", help="Input files/directories to add." ) - add_parser.set_defaults(func=CmdAdd) + parser.set_defaults(func=CmdAdd) From 628229e7a49e253ee3f442c9b751a97eb7d8e285 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Tue, 22 Oct 2019 11:22:25 +0100 Subject: [PATCH 18/22] tidy exception - fixes https://github.com/iterative/dvc/pull/2546#discussion_r337434142 --- dvc/command/add.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 170bc13171..a1f40d7b89 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -25,8 +25,8 @@ def run(self): progress=True, ) - except DvcException as err: - logger.exception("{}:{}".format(type(err).__name__, err)) + except DvcException: + logger.exception("") return 1 return 0 From 7e47abdceccc5c50a26e39039948b4994a675e7a Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Tue, 22 Oct 2019 11:24:42 +0100 Subject: [PATCH 19/22] minor doc update - fixes https://github.com/iterative/dvc/pull/2546/files#r337294491 - related to https://github.com/iterative/dvc.org/issues/719 --- dvc/command/add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index a1f40d7b89..da037d0850 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -32,7 +32,7 @@ def run(self): def add_parser(subparsers, parent_parser): - add_help = "Add data files or directories to DVC control." + add_help = "Track data files or directories with DVC." parser = subparsers.add_parser( "add", From 78933a04ec73b98edad4cb9116479a80d597d49b Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Tue, 22 Oct 2019 11:26:25 +0100 Subject: [PATCH 20/22] Adding => add - fixes https://github.com/iterative/dvc/pull/2546#discussion_r337037137 --- dvc/repo/add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index f7381e853a..3c779804bc 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -29,7 +29,7 @@ def add( stages_list = [] with Tqdm( total=len(targets), - desc="Adding", + desc="Add", unit="file", disable=not progress, leave=True, From 1fffaa810b98610e76471d610e255429bd49f137 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Wed, 23 Oct 2019 14:29:52 +0100 Subject: [PATCH 21/22] various review fixes - minor comment as per https://github.com/iterative/dvc/pull/2546#discussion_r337927837 - revert warning style fix https://github.com/iterative/dvc/pull/2546#discussion_r338046035 - Abstract error consistency https://github.com/iterative/dvc/pull/2546#discussion_r337931070 - minor comment as per https://github.com/iterative/dvc/pull/2546#discussion_r338254958 --- dvc/command/add.py | 6 +++--- dvc/command/base.py | 2 +- dvc/repo/add.py | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index da037d0850..4284c55125 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -32,13 +32,13 @@ def run(self): def add_parser(subparsers, parent_parser): - add_help = "Track data files or directories with DVC." + ADD_HELP = "Track data files or directories with DVC." parser = subparsers.add_parser( "add", parents=[parent_parser], - description=append_doc_link(add_help, "add"), - help=add_help, + description=append_doc_link(ADD_HELP, "add"), + help=ADD_HELP, formatter_class=argparse.RawDescriptionHelpFormatter, ) parser.add_argument( diff --git a/dvc/command/base.py b/dvc/command/base.py index e17ba06f38..f7db28803b 100644 --- a/dvc/command/base.py +++ b/dvc/command/base.py @@ -57,7 +57,7 @@ def default_targets(self): # Abstract methods that have to be implemented by any inheritance class def run(self): - raise NotImplementedError("Abstract") + raise NotImplementedError class CmdBaseNoRepo(CmdBase): diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 3c779804bc..a8d3cfbff2 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -64,6 +64,7 @@ def add( stage.dump() stages_list += stages + # remove filled bar bit of progress, leaving stats pbar.bar_format = pbar.BAR_FMT_DEFAULT.replace("|{bar:10}|", " ") return stages_list From f02951fedc251237c091606422db61341741c028 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 24 Oct 2019 00:03:21 +0100 Subject: [PATCH 22/22] remove diabling progress option - fixes https://github.com/iterative/dvc/pull/2546#discussion_r338314050 --- dvc/command/add.py | 1 - dvc/repo/add.py | 12 ++---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/dvc/command/add.py b/dvc/command/add.py index 4284c55125..788412fb2b 100644 --- a/dvc/command/add.py +++ b/dvc/command/add.py @@ -22,7 +22,6 @@ def run(self): recursive=self.args.recursive, no_commit=self.args.no_commit, fname=self.args.file, - progress=True, ) except DvcException: diff --git a/dvc/repo/add.py b/dvc/repo/add.py index a8d3cfbff2..e81825b0a1 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -17,9 +17,7 @@ @locked @scm_context -def add( - repo, targets, recursive=False, no_commit=False, fname=None, progress=False -): +def add(repo, targets, recursive=False, no_commit=False, fname=None): if recursive and fname: raise RecursiveAddingWhileUsingFilename() @@ -27,13 +25,7 @@ def add( targets = [targets] stages_list = [] - with Tqdm( - total=len(targets), - desc="Add", - unit="file", - disable=not progress, - leave=True, - ) as pbar: + with Tqdm(total=len(targets), desc="Add", unit="file", leave=True) as pbar: for target in targets: sub_targets = _find_all_targets(repo, target, recursive) pbar.total += len(sub_targets) - 1