From ae3455141ca3d2ca607f7ec595cdd97120a029bd Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 22 Jan 2020 19:47:57 -0600 Subject: [PATCH 1/6] get: language feedback on --show-url PR per https://github.com/iterative/dvc/pull/3156#pullrequestreview-345562608 --- dvc/api.py | 4 ++-- dvc/command/get.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dvc/api.py b/dvc/api.py index e1f87d09c1..1707b7338c 100644 --- a/dvc/api.py +++ b/dvc/api.py @@ -47,11 +47,11 @@ class UrlNotDvcRepoError(DvcException): """Thrown if given url is not a DVC repository. Args: - url (str): url to the repository. + url (str): URL to the repository """ def __init__(self, url): - super().__init__("URL '{}' is not a dvc repository.".format(url)) + super().__init__("URL '{}' is not a DVC repository.".format(url)) def get_url(path, repo=None, rev=None, remote=None): diff --git a/dvc/command/get.py b/dvc/command/get.py index 0771bfb261..9d19d594f6 100644 --- a/dvc/command/get.py +++ b/dvc/command/get.py @@ -19,7 +19,7 @@ def _show_url(self): ) logger.info(url) except DvcException: - logger.exception("failed to show url") + logger.exception("Failed to show URL") return 1 return 0 @@ -78,6 +78,6 @@ def add_parser(subparsers, parent_parser): get_parser.add_argument( "--show-url", action="store_true", - help="Returns path/url to the location in remote for specified path", + help="Returns URL to the storage location of the target data.", ) get_parser.set_defaults(func=CmdGet) From ee0193ccd36fdc067ace131958f7960819202643 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 22 Jan 2020 20:12:19 -0600 Subject: [PATCH 2/6] get: rewrite --show-url description per https://github.com/iterative/dvc.org/pull/936#discussion_r368680473 --- dvc/command/get.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dvc/command/get.py b/dvc/command/get.py index 9d19d594f6..8234efad55 100644 --- a/dvc/command/get.py +++ b/dvc/command/get.py @@ -78,6 +78,7 @@ def add_parser(subparsers, parent_parser): get_parser.add_argument( "--show-url", action="store_true", - help="Returns URL to the storage location of the target data.", + help="Just print the storage location (URL) the target data would be " + "downloaded from.", ) get_parser.set_defaults(func=CmdGet) From ec0c39a28ec8a26b76843a715449d54d810fcbb3 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Wed, 22 Jan 2020 20:27:01 -0600 Subject: [PATCH 3/6] term: review capitalization and backquotes around "dvc" rel: https://github.com/iterative/dvc/pull/3156#discussion_r369897073 --- dvc/__main__.py | 2 +- dvc/api.py | 2 +- dvc/cache.py | 4 ++-- dvc/cli.py | 4 ++-- dvc/command/cache.py | 3 ++- dvc/command/daemon.py | 3 ++- dvc/command/init.py | 4 ++-- dvc/command/install.py | 2 +- dvc/command/metrics.py | 2 +- dvc/command/pipeline.py | 2 +- dvc/command/remote.py | 2 +- dvc/config.py | 2 +- dvc/exceptions.py | 4 ++-- dvc/lock.py | 6 +++--- dvc/logger.py | 2 +- dvc/progress.py | 2 +- dvc/scm/__init__.py | 2 +- dvc/stage.py | 2 +- tests/dir_helpers.py | 4 ++-- tests/func/test_api.py | 2 +- 20 files changed, 29 insertions(+), 27 deletions(-) diff --git a/dvc/__main__.py b/dvc/__main__.py index ccd25dc7c8..e9bf8229ef 100644 --- a/dvc/__main__.py +++ b/dvc/__main__.py @@ -1,4 +1,4 @@ -"""Main entry point for dvc command line tool.""" +"""Main entry point for DVC command line tool.""" import sys from dvc.main import main diff --git a/dvc/api.py b/dvc/api.py index 1707b7338c..2ae39bae7c 100644 --- a/dvc/api.py +++ b/dvc/api.py @@ -51,7 +51,7 @@ class UrlNotDvcRepoError(DvcException): """ def __init__(self, url): - super().__init__("URL '{}' is not a DVC repository.".format(url)) + super().__init__("'{}' is not a DVC repository.".format(url)) def get_url(path, repo=None, rev=None, remote=None): diff --git a/dvc/cache.py b/dvc/cache.py index e7e1d6fdcf..0352645024 100644 --- a/dvc/cache.py +++ b/dvc/cache.py @@ -1,4 +1,4 @@ -"""Manages cache of a dvc repo.""" +"""Manages cache of a DVC repo.""" import os from collections import defaultdict @@ -58,7 +58,7 @@ def getter(self): class Cache(object): - """Class that manages cache locations of a dvc repo. + """Class that manages cache locations of a DVC repo. Args: repo (dvc.repo.Repo): repo instance that this cache belongs to. diff --git a/dvc/cli.py b/dvc/cli.py index d4e8045e1c..37d148d7c8 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -1,4 +1,4 @@ -"""Command line interface for dvc.""" +"""DVC command line interface""" import argparse import logging import sys @@ -170,7 +170,7 @@ def parse_args(argv=None): title="Available Commands", metavar="COMMAND", dest="cmd", - help="Use dvc COMMAND --help for command-specific help.", + help="Use `dvc COMMAND --help` for command-specific help.", ) fix_subparsers(subparsers) diff --git a/dvc/command/cache.py b/dvc/command/cache.py index 62b64e4da2..d99ea01a9b 100644 --- a/dvc/command/cache.py +++ b/dvc/command/cache.py @@ -30,7 +30,8 @@ def add_parser(subparsers, parent_parser): ) cache_subparsers = cache_parser.add_subparsers( - dest="cmd", help="Use dvc cache CMD --help for command-specific help." + dest="cmd", + help="Use `dvc cache CMD --help` for command-specific " "help.", ) fix_subparsers(cache_subparsers) diff --git a/dvc/command/daemon.py b/dvc/command/daemon.py index 3089784159..b5cbbfb78b 100644 --- a/dvc/command/daemon.py +++ b/dvc/command/daemon.py @@ -44,7 +44,8 @@ def add_parser(subparsers, parent_parser): ) daemon_subparsers = daemon_parser.add_subparsers( - dest="cmd", help="Use dvc daemon CMD --help for command-specific help." + dest="cmd", + help="Use `dvc daemon CMD --help` for command-specific " "help.", ) fix_subparsers(daemon_subparsers) diff --git a/dvc/command/init.py b/dvc/command/init.py index 671a49c700..35dfc51542 100644 --- a/dvc/command/init.py +++ b/dvc/command/init.py @@ -19,7 +19,7 @@ def run(self): ) self.config = self.repo.config except InitError: - logger.exception("failed to initiate dvc") + logger.exception("Failed to initiate DVC") return 1 return 0 @@ -43,7 +43,7 @@ def add_parser(subparsers, parent_parser): "--no-scm", action="store_true", default=False, - help="Initiate dvc in directory that is " + help="Initiate DVC in directory that is " "not tracked by any scm tool (e.g. git).", ) init_parser.add_argument( diff --git a/dvc/command/install.py b/dvc/command/install.py index b86abc476a..6df33e45a0 100644 --- a/dvc/command/install.py +++ b/dvc/command/install.py @@ -13,7 +13,7 @@ def run(self): try: self.repo.install() except Exception: - logger.exception("failed to install dvc hooks") + logger.exception("Failed to install DVC Git hooks") return 1 return 0 diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index ef79392fbc..3438202a9d 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -171,7 +171,7 @@ def add_parser(subparsers, parent_parser): metrics_subparsers = metrics_parser.add_subparsers( dest="cmd", - help="Use dvc metrics CMD --help to display command-specific help.", + help="Use `dvc metrics CMD --help` to display command-specific help.", ) fix_subparsers(metrics_subparsers) diff --git a/dvc/command/pipeline.py b/dvc/command/pipeline.py index cbec6abf31..eaae094e41 100644 --- a/dvc/command/pipeline.py +++ b/dvc/command/pipeline.py @@ -172,7 +172,7 @@ def add_parser(subparsers, parent_parser): pipeline_subparsers = pipeline_parser.add_subparsers( dest="cmd", - help="Use dvc pipeline CMD --help for command-specific help.", + help="Use `dvc pipeline CMD --help` for command-specific help.", ) fix_subparsers(pipeline_subparsers) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index 0655d440c5..7d3ce773f8 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -83,7 +83,7 @@ def add_parser(subparsers, parent_parser): remote_subparsers = remote_parser.add_subparsers( dest="cmd", - help="Use dvc remote CMD --help for " "command-specific help.", + help="Use `dvc remote CMD --help` for " "command-specific help.", ) fix_subparsers(remote_subparsers) diff --git a/dvc/config.py b/dvc/config.py index 0759395d04..7fac5ef73c 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -76,7 +76,7 @@ def Choices(*choices): class Config(object): # pylint: disable=too-many-instance-attributes - """Class that manages configuration files for a dvc repo. + """Class that manages configuration files for a DVC repo. Args: dvc_dir (str): optional path to `.dvc` directory, that is used to diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 3d577db31c..530f2fe6b9 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -112,7 +112,7 @@ def __init__(self, path): class NotDvcRepoError(DvcException): - """Thrown if a directory is not a dvc repo. + """Thrown if a directory is not a DVC repo. Args: root (str): path to the directory. @@ -120,7 +120,7 @@ class NotDvcRepoError(DvcException): def __init__(self, root): msg = ( - "you are not inside of a dvc repository " + "you are not inside of a DVC repository " "(checked up to mount point '{}')" ) super().__init__(msg.format(root)) diff --git a/dvc/lock.py b/dvc/lock.py index b9c5b951d1..a9d39b46e7 100644 --- a/dvc/lock.py +++ b/dvc/lock.py @@ -22,11 +22,11 @@ class LockError(DvcException): - """Thrown when unable to acquire the lock for dvc repo.""" + """Thrown when unable to acquire the lock for DVC repo.""" class Lock(object): - """Class for dvc repo lock. + """Class for DVC repo lock. Uses zc.lockfile as backend. """ @@ -80,7 +80,7 @@ def __exit__(self, typ, value, tbck): class HardlinkLock(flufl.lock.Lock): - """Class for dvc repo lock. + """Class for DVC repo lock. Args: lockfile (str): the lock filename diff --git a/dvc/logger.py b/dvc/logger.py index 77962fe9e5..9385a6aa2a 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -1,4 +1,4 @@ -"""Manages logging configuration for dvc repo.""" +"""Manages logging configuration for DVC repo.""" import traceback import logging.config diff --git a/dvc/progress.py b/dvc/progress.py index f3c568923d..fefbd98f61 100644 --- a/dvc/progress.py +++ b/dvc/progress.py @@ -1,4 +1,4 @@ -"""Manages progress bars for dvc repo.""" +"""Manages progress bars for DVC repo.""" import logging import sys diff --git a/dvc/scm/__init__.py b/dvc/scm/__init__.py index 73d89fbd16..b6b867232b 100644 --- a/dvc/scm/__init__.py +++ b/dvc/scm/__init__.py @@ -16,7 +16,7 @@ def SCM(root_dir): # pylint: disable=invalid-name Args: root_dir (str): path to a root directory of the repo. - repo (dvc.repo.Repo): dvc repo instance that root_dir belongs to. + repo (dvc.repo.Repo): DVC repo instance that root_dir belongs to. Returns: dvc.scm.base.Base: SCM instance. diff --git a/dvc/stage.py b/dvc/stage.py index 28076bad6d..d63e9dfb8a 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -76,7 +76,7 @@ class StageFileBadNameError(DvcException): class StagePathOutsideError(DvcException): def __init__(self, path): - msg = "stage working or file path '{}' is outside of dvc repo" + msg = "stage working or file path '{}' is outside of DVC repo" super().__init__(msg.format(path)) diff --git a/tests/dir_helpers.py b/tests/dir_helpers.py index 8f0f2969fb..4fc44e2110 100644 --- a/tests/dir_helpers.py +++ b/tests/dir_helpers.py @@ -1,11 +1,11 @@ """ The goal of this module is making dvc functional tests setup a breeze. This -includes a temporary dir, initializing git and dvc repos and bootstrapping some +includes a temporary dir, initializing git and DVC repos and bootstrapping some file structure. The cornerstone of these fixtures is `tmp_dir`, which creates a temporary dir and changes path to it, it might be combined with `scm` and `dvc` to initialize -empty git and dvc repos. `tmp_dir` returns a Path instance, which should save +empty git and DVC repos. `tmp_dir` returns a Path instance, which should save you from using `open()`, `os` and `os.path` utils many times: (tmp_dir / "some_file").write_text("some text") diff --git a/tests/func/test_api.py b/tests/func/test_api.py index 931c1a8f71..687476d619 100644 --- a/tests/func/test_api.py +++ b/tests/func/test_api.py @@ -54,7 +54,7 @@ def test_get_url_external(erepo_dir, remote_url): def test_get_url_requires_dvc(tmp_dir, scm): tmp_dir.scm_gen({"foo": "foo"}, commit="initial") - with pytest.raises(NotDvcRepoError, match="not inside of a dvc repo"): + with pytest.raises(NotDvcRepoError, match="not inside of a DVC repo"): api.get_url("foo", repo=fspath(tmp_dir)) with pytest.raises(UrlNotDvcRepoError): From d06bd6abd8d7e58122890c1676871073ed6cd714 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 23 Jan 2020 00:26:00 -0600 Subject: [PATCH 4/6] tests: match recent commits and other misc. language improvements --- dvc/exceptions.py | 2 +- dvc/remote/config.py | 2 +- tests/func/test_get.py | 4 ++-- tests/func/test_remote.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 530f2fe6b9..c40677170e 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -38,7 +38,7 @@ def __init__(self, output, repo=None): self.output = output self.repo = repo super().__init__( - "unable to find DVC-file with output '{path}'".format( + "Unable to find DVC-file with output '{path}'".format( path=relpath(self.output) ) ) diff --git a/dvc/remote/config.py b/dvc/remote/config.py index 26c023ee12..d97117ac3e 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -33,7 +33,7 @@ def get_settings(self, name): if settings is None: raise ConfigError( - "unable to find remote section '{}'".format(name) + "Unable to find remote section '{}'".format(name) ) parsed = urlparse(settings["url"]) diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 9c69d66aed..efc3c13519 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -190,7 +190,7 @@ def test_get_url_not_existing(tmp_dir, erepo_dir, caplog): main(["get", fspath(erepo_dir), "not-existing-file", "--show-url"]) == 1 ) - assert "failed to show url" in caplog.text + assert "Failed to show URL" in caplog.text def test_get_url_git_only_repo(tmp_dir, scm, caplog): @@ -198,4 +198,4 @@ def test_get_url_git_only_repo(tmp_dir, scm, caplog): with caplog.at_level(logging.ERROR): assert main(["get", fspath(tmp_dir), "foo", "--show-url"]) == 1 - assert "failed to show url" in caplog.text + assert "Failed to show URL" in caplog.text diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py index c6d1cffe9e..4a4ce4b306 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -255,5 +255,5 @@ def test_raise_on_too_many_open_files(tmp_dir, dvc, tmp_path_factory, mocker): def test_modify_missing_remote(dvc): remote_config = RemoteConfig(dvc.config) - with pytest.raises(ConfigError, match=r"unable to find remote section"): + with pytest.raises(ConfigError, match=r"Unable to find remote section"): remote_config.modify("myremote", "gdrive_client_id", "xxx") From 6364c9ee6210effc1d1f53670979872fc1e1ad73 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 23 Jan 2020 00:39:10 -0600 Subject: [PATCH 5/6] get: refine --show-url description per https://github.com/iterative/dvc/pull/3220#pullrequestreview-347038086 --- dvc/command/get.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/command/get.py b/dvc/command/get.py index 8234efad55..76e50ee82e 100644 --- a/dvc/command/get.py +++ b/dvc/command/get.py @@ -78,7 +78,7 @@ def add_parser(subparsers, parent_parser): get_parser.add_argument( "--show-url", action="store_true", - help="Just print the storage location (URL) the target data would be " - "downloaded from.", + help="Print the storage location (URL) the target data would be " + "downloaded from, and exit.", ) get_parser.set_defaults(func=CmdGet) From 69c1aa5bbf3b08ca143afe73894499a7a6121482 Mon Sep 17 00:00:00 2001 From: Jorge Orpinel Date: Thu, 23 Jan 2020 00:50:24 -0600 Subject: [PATCH 6/6] dvc: revert capitalization of logger.exception strings per https://github.com/iterative/dvc/pull/3220#pullrequestreview-347083819 --- dvc/command/get.py | 2 +- dvc/command/init.py | 2 +- dvc/command/install.py | 2 +- tests/func/test_get.py | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dvc/command/get.py b/dvc/command/get.py index 76e50ee82e..e27c178b55 100644 --- a/dvc/command/get.py +++ b/dvc/command/get.py @@ -19,7 +19,7 @@ def _show_url(self): ) logger.info(url) except DvcException: - logger.exception("Failed to show URL") + logger.exception("failed to show URL") return 1 return 0 diff --git a/dvc/command/init.py b/dvc/command/init.py index 35dfc51542..fe0951d488 100644 --- a/dvc/command/init.py +++ b/dvc/command/init.py @@ -19,7 +19,7 @@ def run(self): ) self.config = self.repo.config except InitError: - logger.exception("Failed to initiate DVC") + logger.exception("failed to initiate DVC") return 1 return 0 diff --git a/dvc/command/install.py b/dvc/command/install.py index 6df33e45a0..1dea3fcb07 100644 --- a/dvc/command/install.py +++ b/dvc/command/install.py @@ -13,7 +13,7 @@ def run(self): try: self.repo.install() except Exception: - logger.exception("Failed to install DVC Git hooks") + logger.exception("failed to install DVC Git hooks") return 1 return 0 diff --git a/tests/func/test_get.py b/tests/func/test_get.py index efc3c13519..45f8393551 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -190,7 +190,7 @@ def test_get_url_not_existing(tmp_dir, erepo_dir, caplog): main(["get", fspath(erepo_dir), "not-existing-file", "--show-url"]) == 1 ) - assert "Failed to show URL" in caplog.text + assert "failed to show URL" in caplog.text def test_get_url_git_only_repo(tmp_dir, scm, caplog): @@ -198,4 +198,4 @@ def test_get_url_git_only_repo(tmp_dir, scm, caplog): with caplog.at_level(logging.ERROR): assert main(["get", fspath(tmp_dir), "foo", "--show-url"]) == 1 - assert "Failed to show URL" in caplog.text + assert "failed to show URL" in caplog.text