From cf7bafad4dbc095e78a2fdd9e70390fb85e838fb Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sun, 14 Jan 2018 17:18:20 +0200 Subject: [PATCH 1/2] dvc: remove brancher and Find Fixes #383 Fixes #376 We discussed this with @dmpetrov and decided to remove this ml-specific functionality from the core product. Signed-off-by: Ruslan Kuprieiev --- dvc/cli.py | 40 +----------------- dvc/command/common/base.py | 1 - dvc/command/common/branch_changer.py | 31 -------------- dvc/command/find.py | 61 ---------------------------- dvc/project.py | 1 - dvc/scm.py | 4 -- tests/test_cli.py | 21 ---------- 7 files changed, 1 insertion(+), 158 deletions(-) delete mode 100644 dvc/command/common/branch_changer.py delete mode 100644 dvc/command/find.py diff --git a/dvc/cli.py b/dvc/cli.py index fda8c4bef..1aaf42f9a 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -5,7 +5,6 @@ import sys import argparse from multiprocessing import cpu_count -from dvc.command.find import CmdFind from dvc.command.fsck import CmdFsck from dvc.command.init import CmdInit from dvc.command.remove import CmdRemove @@ -38,17 +37,6 @@ def parse_args(argv=None): default=False, help='Be verbose.') - parent_parser.add_argument( - '-b', - '--branch', - metavar='BRANCH', - help='Execute a command in the branch.') - parent_parser.add_argument( - '-n', - '--new-branch', - metavar='BRANCH', - help='Create a new branch and execute a command in the branch.') - # Main parser desc = 'Data Version Control' parser = argparse.ArgumentParser( @@ -272,33 +260,7 @@ def parse_args(argv=None): help='Checkout') checkout_parser.set_defaults(func=CmdCheckout) - # Find - find_parser = subparsers.add_parser( - 'find', - parents=[parent_parser], - help='Find branch name') - find_parser.set_defaults(func=CmdFind) - find_parser.add_argument( - 'branch_name', - nargs='?', - help='Branch name regexp.') - find_parser.add_argument( - 'target', - nargs='?', - help='Target metric file.') - find_parser.add_argument( - '-c', - '--criteria', - help='Search criteria. By default it finds max.', - choices=['max', 'min', 'all'], - default='max') - find_parser.add_argument( - '-s', - '--show-value', - action='store_true', - default=False, - help='Show metrics value.') - + # Fsck fsck_parser = subparsers.add_parser( 'fsck', parents=[parent_parser], diff --git a/dvc/command/common/base.py b/dvc/command/common/base.py index 2907b5d9e..b6ba891dd 100644 --- a/dvc/command/common/base.py +++ b/dvc/command/common/base.py @@ -27,7 +27,6 @@ class CmdBase(object): def run_cmd(self): with self.project.lock: - with self.project.scm.brancher(self.args.branch, self.args.new_branch): return self.run() # Abstract methods that have to be implemented by any inheritance class diff --git a/dvc/command/common/branch_changer.py b/dvc/command/common/branch_changer.py deleted file mode 100644 index 94e6cf420..000000000 --- a/dvc/command/common/branch_changer.py +++ /dev/null @@ -1,31 +0,0 @@ -from dvc.command.common.common_error import CmdCommonError - - -class BranchChangerError(CmdCommonError): - def __init__(self, branch, msg): - super(CmdCommonError, self).__init__('Error in changing branch \'{}\': {}'.format(branch, msg)) - - -class BranchChanger(object): - def __init__(self, scm, branch, new_branch): - if branch and new_branch: - raise BranchChangerError("Commands conflict: --branch and --new-branch cannot be used at the same command") - - self.perform_action = branch or new_branch - - self.branch = branch if branch else new_branch - self.create_new = new_branch is not None - self.scm = scm - - def __enter__(self): - if self.perform_action: - code, _, err = self.scm.checkout(self.branch, self.create_new) - if code != 0: - raise BranchChangerError(self.branch, err) - return self - - def __exit__(self, type, value, traceback): - if self.perform_action: - code, _, err = self.scm.checkout('-') - if code != 0: - raise BranchChangerError(self.branch, err) diff --git a/dvc/command/find.py b/dvc/command/find.py deleted file mode 100644 index 3bb708c02..000000000 --- a/dvc/command/find.py +++ /dev/null @@ -1,61 +0,0 @@ -from dvc import utils -from dvc.command.common.base import CmdBase -from dvc.executor import ExecutorError -from dvc.logger import Logger - - -class CmdFind(CmdBase): - def __init__(self, settings): - super(CmdFind, self).__init__(settings) - - def run(self): - fname = self.parsed_args.target - branch_name = self.parsed_args.branch_name - - branches = self.git.branches(branch_name) - metrics = [self.read_metrics(fname, b) for b in branches] - - zipped = zip(metrics, branches) - res = self.filter(zipped, self.parsed_args.criteria) - res = filter(lambda x: x[0] is not None, res) - - for (val, branch) in res: - if self.parsed_args.show_value: - print('{}: {}'.format(branch, val)) - else: - print(branch) - return 0 - - @staticmethod - def filter(zipped, criteria): - if criteria == 'max': - res = [max(zipped, key=lambda t: t[0])] - elif criteria == 'min': - res = [min(zipped, key=lambda t: t[0])] - else: - res = zipped - return res - - def read_metrics(self, fname, branch): - try: - lines = self.git.get_file_content(fname, branch).split('\n') - except ExecutorError as ex: - msg = 'Unable to read metrics file from branch {}: {}' - data_item = self.settings.path_factory.data_item(fname) - - try: - self.git.get_file_content(data_item.state.relative, branch) - Logger.error(msg.format(branch, 'this is data file, not metric file')) - except ExecutorError: - Logger.error(msg.format(branch, 'file does not exist in this branch')) - return None - - return None - - metric = utils.parse_target_metric(lines) - if not metric: - msg = 'Unable to parse metrics from the first line of file {} in branch {}' - Logger.error(msg.format(fname, branch)) - return None - - return metric diff --git a/dvc/project.py b/dvc/project.py index c6c076662..61b7c3585 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -109,7 +109,6 @@ class Project(object): ignore_file = os.path.join(dvc_dir, scm.ignore_file()) scm.add([config.config_file, ignore_file]) - scm.commit('DVC init') return Project(root_dir) diff --git a/dvc/scm.py b/dvc/scm.py index acc333cc6..71eaecf0e 100644 --- a/dvc/scm.py +++ b/dvc/scm.py @@ -2,7 +2,6 @@ import os import git from dvc.exceptions import DvcException -from dvc.command.common.branch_changer import BranchChanger class SCMError(DvcException): @@ -45,9 +44,6 @@ class Base(object): def branch(self, branch): pass - def brancher(self, branch, new_branch): - return BranchChanger(self, branch, new_branch) - def untracked_files(self): pass diff --git a/tests/test_cli.py b/tests/test_cli.py index 143c68c1b..b65affd93 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -12,7 +12,6 @@ from dvc.command.config import CmdConfig from dvc.command.show_pipeline import CmdShowPipeline from dvc.command.show_workflow import CmdShowWorkflow from dvc.command.checkout import CmdCheckout -from dvc.command.find import CmdFind from dvc.command.fsck import CmdFsck from tests.basic_env import TestDvc @@ -184,26 +183,6 @@ class TestCheckout(TestDvc): self.assertIsInstance(args.func(args), CmdCheckout) -class TestFind(TestDvc): - def test(self): - branch = 'branch' - target = 'target' - criteria = 'max' - - args = parse_args(['find', - branch, - target, - '-c', criteria, - '--criteria', criteria, - '-s', '--show-value']) - - self.assertIsInstance(args.func(args), CmdFind) - self.assertEqual(args.branch_name, branch) - self.assertEqual(args.target, target) - self.assertEqual(args.criteria, criteria) - self.assertTrue(args.show_value) - - class TestFsck(TestDvc): def test(self): target1 = 'target1' From 266a908169d30d5e9c8aaed4235e7117b51d84c6 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sun, 14 Jan 2018 18:38:54 +0200 Subject: [PATCH 2/2] dvc: fix gitignore bug Fixes #375 Signed-off-by: Ruslan Kuprieiev --- dvc/scm.py | 5 +++-- tests/test_scm.py | 22 +++++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/dvc/scm.py b/dvc/scm.py index 71eaecf0e..2c8cdf78e 100644 --- a/dvc/scm.py +++ b/dvc/scm.py @@ -80,11 +80,12 @@ class Git(Base): ignore_list = [] if os.path.exists(gitignore): ignore_list = open(gitignore, 'r').readlines() - if entry in ignore_list: + filtered = list(filter(lambda x: x.strip() == entry.strip(), ignore_list)) + if len(filtered) != 0: return content = entry - if ignore_list: + if len(ignore_list) > 0: content = '\n' + content open(gitignore, 'a').write(content) diff --git a/tests/test_scm.py b/tests/test_scm.py index 70fb50bb6..f3aaeed47 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -20,13 +20,25 @@ class TestSCMGit(TestGit): def test_is_repo(self): self.assertTrue(Git.is_repo(os.curdir)) - def test_ignore(self): - Git(self._root_dir).ignore('foo') - self.assertTrue(os.path.isfile(Git.GITIGNORE)) - self.assertTrue('foo' in open(Git.GITIGNORE, 'r').readlines()) - def test_commit(self): G = Git(self._root_dir) G.add(['foo']) G.commit('add') self.assertTrue('foo' in self.git.git.ls_files()) + + +class TestIgnore(TestGit): + def _count_gitignore(self): + with open(Git.GITIGNORE, 'r') as fd: + lines = fd.readlines() + return len(list(filter(lambda x: x.strip() == 'foo', lines))) + + def test(self): + git = Git(self._root_dir) + + git.ignore('foo') + self.assertTrue(os.path.isfile(Git.GITIGNORE)) + self.assertEqual(self._count_gitignore(), 1) + + git.ignore('foo') + self.assertEqual(self._count_gitignore(), 1)