Final changes and unit-tests fixes

This commit is contained in:
Dmitry
2017-09-16 22:11:32 -07:00
parent b3f70d641e
commit 563ad0f513
5 changed files with 91 additions and 70 deletions
+7 -7
View File
@@ -417,23 +417,23 @@ def parse_args(argv=None):
workflow_parser = show_subparsers.add_parser(
'workflow',
parents=[parent_parser],
help='Show workflow image')
help='Show workflow image. It collapses DVC repro commits if possible.')
workflow_parser.add_argument(
'target',
nargs='?',
help='Target metric data file')
workflow_parser.add_argument(
'-d',
'--dvc-commits',
action='store_true',
default=False,
help='Show DVC repro commits.')
workflow_parser.add_argument(
'-a',
'--all-commits',
action='store_true',
default=False,
help='Show all commits')
workflow_parser.add_argument(
'-d',
'--deltas',
action='store_true',
default=False,
help='Show only metric deltas')
workflow_parser.set_defaults(func=CmdShowWorkflow)
+1 -1
View File
@@ -21,7 +21,7 @@ class CmdShowWorkflow(CmdBase):
Logger.debug(u'Set show workflow target as {}'.format(target))
wf = self.git.get_all_commits(target, self.settings)
wf.build_graph(self.settings.parsed_args.all_commits, self.settings.parsed_args.deltas)
wf.build_graph(self.settings.parsed_args.dvc_commits, self.settings.parsed_args.all_commits)
return 0
@property
+18 -16
View File
@@ -41,20 +41,17 @@ class Commit(object):
def text(self):
branch_text = ''
if self._branch_tips:
branch_text = 'Branch tips: {}\n'.format(', '.join(self._branch_tips))
branch_text = 'BRANCH TIPS: {}\n'.format(', '.join(self._branch_tips))
# text = self._text_comment(self.hash, self._comment, self._is_collapsed)
text = ''
return self._text_metrics_line() + branch_text + text + self._text_added_comments
return self._text_metrics_line() + branch_text + self._comments_text
def _text_hash(self):
return 'Commit: ' + self.hash
# @property
@staticmethod
def _text_comment(hash, comment, is_collapsed):
text = Commit.COLLAPSED_TEXT if is_collapsed else Commit._limit_text(comment)
return '{}: {}'.format(hash, text)
def _text_comment(commit):
if commit._is_collapsed:
text = Commit.COLLAPSED_TEXT
else:
text = Commit._limit_text(commit._comment)
return '[{}] {}'.format(commit.hash, text)
@staticmethod
def _limit_text(comment):
@@ -64,12 +61,17 @@ class Commit(object):
return comment[:Commit.TEXT_LIMIT-3] + '...'
@property
def _text_added_comments(self):
res = [self._text_comment(self.hash, self._comment, self._is_collapsed)]
def _comments_text(self):
res = []
if not self.is_repro:
res.append(self._text_comment(self))
for commit in self._collapsed_commits:
text = self._text_comment(commit.hash, commit._comment, commit.is_repro)
res.append(text)
if not commit.is_repro:
text = self._text_comment(commit)
res.append(text)
if len(res) == 0:
return self._text_comment(self)
max_len = max(map(lambda x: len(x), res))
res_extended_len = map(lambda x: x.ljust(max_len), res)
@@ -78,7 +80,7 @@ class Commit(object):
def _text_metrics_line(self):
result = ''
if self._is_target and self._target_metric:
result = 'Target: {}'.format(self._target_metric)
result = 'TARGET: {}'.format(self._target_metric)
if self._target_metric_delta is not None:
result += ' ({:+f})'.format(self._target_metric_delta)
result += '\n'
+18 -31
View File
@@ -36,9 +36,9 @@ class CommitCollapseStrategy(object):
child.add_branch_tips(commit.branch_tips)
class CollapseDvcCommitsStrategy(CommitCollapseStrategy):
class CollapseDvcReproCommitsStrategy(CommitCollapseStrategy):
def __init__(self, workflow):
super(CollapseDvcCommitsStrategy, self).__init__(workflow)
super(CollapseDvcReproCommitsStrategy, self).__init__(workflow)
def is_change_needed(self, commit, hash):
return commit.is_repro
@@ -57,7 +57,7 @@ class CollapseNotMeticsCommitsStrategy(CommitCollapseStrategy):
def is_remove_needed(self, commit):
child_commits = self._workflow.child_commits(commit.hash)
return len(child_commits) == 1 and child_commits[0].has_target_metric
return len(child_commits) == 1 and not commit.has_target_metric
def upstream_to_child(self, commit, child):
super(CollapseNotMeticsCommitsStrategy, self).upstream_to_child(commit, child)
@@ -109,19 +109,15 @@ class Workflow(object):
return []
return [self._commits[h] for h in self._back_edges[hash]]
def build_graph(self, all_commits, deltas):
def build_graph(self, show_dvc_commits, show_all_commits):
g = nx.DiGraph(name='DVC Workflow', directed=False)
self.derive_target_metric_deltas()
# if not all_commits:
# self.collapse_commits(CollapseDvcCommitsStrategy(self))
# elif deltas:
# self.collapse_commits(CollapseNotMeticsCommitsStrategy(self))
# self.deltas_only()
self.collapse_commits(CollapseDvcCommitsStrategy(self))
self.collapse_commits(CollapseNotMeticsCommitsStrategy(self))
if not show_all_commits:
self.collapse_commits(CollapseDvcReproCommitsStrategy(self))
if not show_dvc_commits:
self.collapse_commits(CollapseNotMeticsCommitsStrategy(self))
for hash in set(self._edges.keys() + self._back_edges.keys()):
commit = self._commits[hash]
@@ -163,10 +159,6 @@ class Workflow(object):
self._build_graph(next, g)
def deltas_only(self):
pass
def collapse_commits(self, strategy):
hashes_to_remove = []
for commit in self._commits.values():
@@ -176,15 +168,17 @@ class Workflow(object):
for hash in hashes_to_remove:
del self._commits[hash]
del self._edges[hash]
del self._back_edges[hash]
if hash in self._edges:
del self._edges[hash]
if hash in self._back_edges:
del self._back_edges[hash]
pass
def _remove_or_collapse(self, commit, strategy):
parent_commit_hashes = self._back_edges.get(commit.hash)
child_commit_hashes = self._edges.get(commit.hash)
if parent_commit_hashes is None or child_commit_hashes is None:
if child_commit_hashes is None:
return False
if strategy.is_remove_needed(commit):
@@ -203,20 +197,13 @@ class Workflow(object):
return False
pass
# def _upstream_metrics_and_branch_tips(self, child_commit_hashes, commit):
# for hash in child_commit_hashes:
# if commit.has_target_metric:
# if not self._commits[hash].has_target_metric:
# self._commits[hash].set_target_metric(commit.target_metric)
# if commit.branch_tips:
# self._commits[hash].add_branch_tips(commit.branch_tips)
# pass
@staticmethod
def _redirect_edges(edges, child_commit_hashes, hash, commit_hashes):
for h in commit_hashes:
edges[h].remove(hash)
edges[h] |= child_commit_hashes
if commit_hashes:
for h in commit_hashes:
edges[h].remove(hash)
if child_commit_hashes:
edges[h] |= child_commit_hashes
def derive_target_metric_deltas(self):
if not self._target:
+47 -15
View File
@@ -1,6 +1,7 @@
from unittest import TestCase
from dvc.graph.workflow import Workflow
from dvc.graph.workflow import Workflow,\
CollapseDvcReproCommitsStrategy, CollapseNotMeticsCommitsStrategy
from dvc.graph.commit import Commit
@@ -11,6 +12,13 @@ class TestWorkflow(TestCase):
self._commit2 = Commit('2', '1', 'name1', 'today', 'DVC repro-run ...')
self._commit1 = Commit('1', '', 'name1', 'today', 'comment1')
self._showed_text1 = '[' + self._commit1.hash + '] ' + self._commit1._comment
self._showed_text2 = '[' + self._commit2.hash + '] ' + self._commit2._comment
self._showed_text3 = '[' + self._commit3.hash + '] ' + self._commit3._comment
self._showed_text4 = '[' + self._commit4.hash + '] ' + self._commit4._comment
self._showed_text3_collapsed = '[' + self._commit3.hash + '] ' + Commit.COLLAPSED_TEXT
def commits_basic_test(self):
self.assertFalse(self._commit1.is_repro)
self.assertTrue(self._commit2.is_repro)
@@ -27,10 +35,10 @@ class TestWorkflow(TestCase):
self.assertEqual(len(wf._commits), 4)
self.assertEqual(wf._commits['1'].text, self._commit1._comment + '\n' + self._commit1.hash)
self.assertEqual(wf._commits['2'].text, self._commit2._comment + '\n' + self._commit2.hash)
self.assertEqual(wf._commits['3'].text, self._commit3._comment + '\n' + self._commit3.hash)
self.assertEqual(wf._commits['4'].text, self._commit4._comment + '\n' + self._commit4.hash)
self.assertEqual(wf._commits['1'].text, self._showed_text1)
self.assertEqual(wf._commits['2'].text, self._showed_text2)
self.assertEqual(wf._commits['3'].text, self._showed_text3)
self.assertEqual(wf._commits['4'].text, self._showed_text4)
pass
def collapse_test(self):
@@ -40,11 +48,11 @@ class TestWorkflow(TestCase):
wf.add_commit(self._commit3)
wf.add_commit(self._commit4)
wf.collapse_commits()
wf.collapse_commits(CollapseDvcReproCommitsStrategy(wf))
self.assertEqual(len(wf._commits), 3)
self.assertEqual(wf._commits[self._commit1.hash].text, self._commit1._comment + '\n' + self._commit1.hash)
self.assertEqual(wf._commits[self._commit3.hash].text, Commit.COLLAPSED_TEXT)
self.assertEqual(wf._commits[self._commit1.hash].text, self._showed_text1)
self.assertEqual(wf._commits[self._commit3.hash].text, self._showed_text3_collapsed)
self.assertTrue('2' not in wf._commits)
self.assertFalse('2' in wf._edges)
@@ -58,29 +66,53 @@ class TestWorkflow(TestCase):
wf.add_commit(self._commit3) # Dead end which cannot be collapsed
self.assertEqual(len(wf._commits), 3)
wf.collapse_commits()
wf.collapse_commits(CollapseDvcReproCommitsStrategy(wf))
self.assertEqual(len(wf._commits), 2)
self.assertEqual(wf._commits[self._commit1.hash].text, self._commit1._comment + '\n' + self._commit1.hash)
self.assertEqual(wf._commits[self._commit3.hash].text, Commit.COLLAPSED_TEXT)
self.assertEqual(wf._commits[self._commit1.hash].text, self._showed_text1)
self.assertEqual(wf._commits[self._commit3.hash].text, self._showed_text3)
self.assertTrue('2' not in wf._commits)
pass
def collapse_metric_commit_test(self):
value = 0.812345
branches = ['master', 'try_smth']
metric_commit3 = Commit('2', '1', 'name1', 'today', 'DVC repro-run ...',
metric_commit2 = Commit('2', '1', 'name1', 'today', 'DVC repro-run ...',
True, value, branch_tips=branches)
wf = Workflow('', '')
wf.add_commit(self._commit1)
wf.add_commit(metric_commit3)
wf.add_commit(metric_commit2)
wf.add_commit(self._commit3)
self.assertEqual(len(wf._commits), 3)
wf.collapse_commits()
wf.collapse_commits(CollapseDvcReproCommitsStrategy(wf))
self.assertEqual(len(wf._commits), 2)
self.assertEqual(wf._commits['3']._target_metric, value)
self.assertEqual(wf._commits['3'].branch_tips, branches)
pass
pass
def collapse_all_commits_test(self):
value = 0.812345
branches = ['master', 'try_smth']
not_metric_commit2 = Commit('2', '1', 'name1', 'today', 'change smth 2...',
True, branch_tips=branches)
metric_commit3 = Commit('3', '2', 'name3', 'today3', 'hello3',
True, value)
wf = Workflow('', '')
wf.add_commit(self._commit1)
wf.add_commit(not_metric_commit2)
wf.add_commit(metric_commit3)
wf.add_commit(self._commit4)
self.assertEqual(len(wf._commits), 4)
wf.collapse_commits(CollapseDvcReproCommitsStrategy(wf))
self.assertEqual(len(wf._commits), 4)
wf.collapse_commits(CollapseNotMeticsCommitsStrategy(wf))
self.assertEqual(len(wf._commits), 2)
self.assertEqual(wf._commits['3']._target_metric, value)
self.assertEqual(wf._commits['3'].branch_tips, branches)
pass