From 0ee2073dbe12d414273bc6a39655fff4dd20e9ee Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 6 Sep 2017 09:55:09 -0700 Subject: [PATCH 1/6] Add cmd options --- dvc/cli.py | 13 +++++++++++++ dvc/command/show_workflow.py | 2 +- dvc/graph/commit.py | 25 +++++++++++++++---------- dvc/graph/workflow.py | 17 +++++++++++------ 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index a86ee86f8..106e840ec 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -422,6 +422,19 @@ def parse_args(argv=None): 'target', nargs='?', help='Target metric data file') + 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) if isinstance(argv, str): diff --git a/dvc/command/show_workflow.py b/dvc/command/show_workflow.py index 83ed1d163..8008939a4 100644 --- a/dvc/command/show_workflow.py +++ b/dvc/command/show_workflow.py @@ -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() + wf.build_graph(self.settings.parsed_args.all_commits, self.settings.parsed_args.deltas) return 0 @property diff --git a/dvc/graph/commit.py b/dvc/graph/commit.py index 79b6e4c66..7ccac14aa 100644 --- a/dvc/graph/commit.py +++ b/dvc/graph/commit.py @@ -1,7 +1,7 @@ class Commit(object): TEXT_LIMIT = 30 DVC_REPRO_PREFIX = 'DVC repro' - COLLAPSED_TEXT = DVC_REPRO_PREFIX + ' <>' + COLLAPSED_TEXT = DVC_REPRO_PREFIX + ' <>' def __init__(self, hash, parents, name, date, comment, is_target=False, @@ -39,32 +39,36 @@ class Commit(object): if self._branch_tips: branch_text = 'Branch tips: {}\n'.format(', '.join(self._branch_tips)) - if self._is_collapsed: - return branch_text + self.COLLAPSED_TEXT + self._text_metrics_line() - - return branch_text + self._text_comment() + self._text_metrics_line() + '\n' + self._text_hash() + return self._text_metrics_line() + branch_text + self._text_comment def _text_hash(self): return 'Commit: ' + self.hash + @property def _text_comment(self): - if len(self._comment) < self.TEXT_LIMIT: - return self._comment - return self._comment[:self.TEXT_LIMIT] + '...' + result = self.hash + ': ' + if self._is_collapsed: + result += self.COLLAPSED_TEXT + elif len(self._comment) < self.TEXT_LIMIT: + result += self._comment + else: + result += self._comment[:self.TEXT_LIMIT] + '...' + return result def _text_metrics_line(self): result = '' if self._is_target and self._target_metric: - result = '\nTarget: {}'.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' return result @property def is_repro(self): return self._comment.startswith(self.DVC_REPRO_PREFIX) - def make_colapsed(self): + def make_collapsed(self): self._is_collapsed = True @property @@ -93,3 +97,4 @@ class Commit(object): @property def target_metric_delta(self): return self._target_metric_delta + diff --git a/dvc/graph/workflow.py b/dvc/graph/workflow.py index f33410b17..1e6e1db87 100644 --- a/dvc/graph/workflow.py +++ b/dvc/graph/workflow.py @@ -10,7 +10,7 @@ class WorkflowError(DvcException): class Workflow(object): - def __init__(self, target, merges_map, branches_map=None, no_repro_commits=True): + def __init__(self, target, merges_map, branches_map=None): self._target = target self._merges_map = merges_map self._branches_map = branches_map @@ -20,8 +20,6 @@ class Workflow(object): self._edges = {} self._back_edges = {} - - self._no_repro_commits = no_repro_commits pass def add_commit(self, commit): @@ -39,13 +37,16 @@ class Workflow(object): self._root_hash = commit.hash pass - def build_graph(self): + def build_graph(self, all_commits, deltas): g = nx.DiGraph(name='DVC Workflow', directed=False) self.derive_target_metric_deltas() - if self._no_repro_commits: + if not all_commits: self.collapse_repro_commits() + elif deltas: + self.collapse_repro_commits() + self.deltas_only() for hash in set(self._edges.keys() + self._back_edges.keys()): commit = self._commits[hash] @@ -87,6 +88,10 @@ class Workflow(object): self._build_graph(next, g) + def deltas_only(self): + + pass + def collapse_repro_commits(self): hashes_to_remove = [] for commit in self._commits.values(): @@ -116,7 +121,7 @@ class Workflow(object): return True else: - commit.make_colapsed() + commit.make_collapsed() return False pass From 95f39de424f353627b0cd0ac877cfb0d7679262d Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 13 Sep 2017 00:58:49 -0700 Subject: [PATCH 2/6] Keep only human commits --- dvc/graph/commit.py | 28 ++++++---- dvc/graph/workflow.py | 120 +++++++++++++++++++++++++++++++++-------- tests/test_workflow.py | 6 +-- 3 files changed, 121 insertions(+), 33 deletions(-) diff --git a/dvc/graph/commit.py b/dvc/graph/commit.py index 7ccac14aa..6fd74c146 100644 --- a/dvc/graph/commit.py +++ b/dvc/graph/commit.py @@ -16,6 +16,7 @@ class Commit(object): self._target_metric = target_metric self._target_metric_delta = None self._branch_tips = [] if branch_tips is None else branch_tips + self._added_commits_pairs = [] self._is_collapsed = False @@ -33,27 +34,37 @@ class Commit(object): def remove_parent(self, hash): self._parent_hashes.remove(hash) + def add_collapsed_commit(self, commit): + self._added_commits_pairs.append((commit.hash, commit._comment)) + @property def text(self): branch_text = '' if self._branch_tips: branch_text = 'Branch tips: {}\n'.format(', '.join(self._branch_tips)) - return self._text_metrics_line() + branch_text + self._text_comment + return self._text_metrics_line() + branch_text + self._text_comment + self._text_added_comments def _text_hash(self): return 'Commit: ' + self.hash @property def _text_comment(self): - result = self.hash + ': ' - if self._is_collapsed: - result += self.COLLAPSED_TEXT - elif len(self._comment) < self.TEXT_LIMIT: - result += self._comment + text = self.COLLAPSED_TEXT if self._is_collapsed else self._limit_text(self._comment) + return '{}: {}'.format(self.hash, text) + + def _limit_text(self, comment): + if len(comment) < self.TEXT_LIMIT: + return self._comment else: - result += self._comment[:self.TEXT_LIMIT] + '...' - return result + return comment[:self.TEXT_LIMIT-3] + '...' + + @property + def _text_added_comments(self): + res = '' + for hash, commit in self._added_commits_pairs: + res += '\n{}: {}'.format(hash, self._limit_text(commit)) + return res def _text_metrics_line(self): result = '' @@ -97,4 +108,3 @@ class Commit(object): @property def target_metric_delta(self): return self._target_metric_delta - diff --git a/dvc/graph/workflow.py b/dvc/graph/workflow.py index 1e6e1db87..4b0bb1914 100644 --- a/dvc/graph/workflow.py +++ b/dvc/graph/workflow.py @@ -9,6 +9,65 @@ class WorkflowError(DvcException): super(WorkflowError, self).__init__('Workflow error: ' + msg) +class CommitCollapseStrategy(object): + def __init__(self, workflow): + self._workflow = workflow + pass + + def is_change_needed(self, commit, child_commits): + raise NotImplementedError() + + def is_remove_needed(self, commit): + raise NotImplementedError() + + def make_collapsed(self, commit): + commit.make_collapsed() + + def upstream(self, commit): + for child in self._workflow.child_commits(commit.hash): + self.upstream_to_child(commit, child) + pass + + def upstream_to_child(self, commit, child): + if commit.has_target_metric: + if not child.has_target_metric: + child.set_target_metric(commit.target_metric) + if commit.branch_tips: + child.add_branch_tips(commit.branch_tips) + + +class CollapseDvcCommitsStrategy(CommitCollapseStrategy): + def __init__(self, workflow): + super(CollapseDvcCommitsStrategy, self).__init__(workflow) + + def is_change_needed(self, commit, hash): + return commit.is_repro + + def is_remove_needed(self, commit): + child_commits = self._workflow.child_commits(commit.hash) + return child_commits and all(ch.is_repro for ch in child_commits) + + +class CollapseNotMeticsCommitsStrategy(CommitCollapseStrategy): + def __init__(self, workflow): + super(CollapseNotMeticsCommitsStrategy, self).__init__(workflow) + + def is_change_needed(self, commit, hash): + return not commit.has_target_metric + + 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 + + def upstream_to_child(self, commit, child): + super(CollapseNotMeticsCommitsStrategy, self).upstream_to_child(commit, child) + if not commit.is_repro: + child.add_collapsed_commit(commit) + + def make_collapsed(self, commit): + pass + + class Workflow(object): def __init__(self, target, merges_map, branches_map=None): self._target = target @@ -37,16 +96,32 @@ class Workflow(object): self._root_hash = commit.hash pass + def get_commit(self, hash): + return self._commits.get(hash) + + def child_commits(self, hash): + if hash not in self._edges: + return [] + return [self._commits[h] for h in self._edges[hash]] + + def parent_commits(self, hash): + if hash not in self._back_edges: + return [] + return [self._commits[h] for h in self._back_edges[hash]] + def build_graph(self, all_commits, deltas): g = nx.DiGraph(name='DVC Workflow', directed=False) self.derive_target_metric_deltas() - if not all_commits: - self.collapse_repro_commits() - elif deltas: - self.collapse_repro_commits() - self.deltas_only() + # 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)) for hash in set(self._edges.keys() + self._back_edges.keys()): commit = self._commits[hash] @@ -92,11 +167,11 @@ class Workflow(object): pass - def collapse_repro_commits(self): + def collapse_commits(self, strategy): hashes_to_remove = [] for commit in self._commits.values(): - if commit.is_repro: - if self._remove_or_collapse(commit): + if strategy.is_change_needed(commit, commit.hash): + if self._remove_or_collapse(commit, strategy): hashes_to_remove.append(commit.hash) for hash in hashes_to_remove: @@ -105,12 +180,15 @@ class Workflow(object): del self._back_edges[hash] pass - def _remove_or_collapse(self, commit): - parent_commit_hashes = self._back_edges[commit.hash] + 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 child_commit_hashes and all(self._commits[hash].is_repro for hash in child_commit_hashes): - self._upstream_metrics_and_branch_tips(child_commit_hashes, commit) + if parent_commit_hashes is None or child_commit_hashes is None: + return False + + if strategy.is_remove_needed(commit): + strategy.upstream(commit) for hash in child_commit_hashes: self._commits[hash].add_parents(commit.parent_hashes) @@ -121,18 +199,18 @@ class Workflow(object): return True else: - commit.make_collapsed() + strategy.make_collapsed(commit) 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 + # 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): diff --git a/tests/test_workflow.py b/tests/test_workflow.py index e497a9f3f..a9a6f0aa5 100644 --- a/tests/test_workflow.py +++ b/tests/test_workflow.py @@ -40,7 +40,7 @@ class TestWorkflow(TestCase): wf.add_commit(self._commit3) wf.add_commit(self._commit4) - wf.collapse_repro_commits() + wf.collapse_commits() self.assertEqual(len(wf._commits), 3) self.assertEqual(wf._commits[self._commit1.hash].text, self._commit1._comment + '\n' + self._commit1.hash) @@ -58,7 +58,7 @@ class TestWorkflow(TestCase): wf.add_commit(self._commit3) # Dead end which cannot be collapsed self.assertEqual(len(wf._commits), 3) - wf.collapse_repro_commits() + wf.collapse_commits() self.assertEqual(len(wf._commits), 2) self.assertEqual(wf._commits[self._commit1.hash].text, self._commit1._comment + '\n' + self._commit1.hash) @@ -78,7 +78,7 @@ class TestWorkflow(TestCase): wf.add_commit(self._commit3) self.assertEqual(len(wf._commits), 3) - wf.collapse_repro_commits() + wf.collapse_commits() self.assertEqual(len(wf._commits), 2) self.assertEqual(wf._commits['3']._target_metric, value) From b3f70d641e5aaa966455feb75a08c622a67b2b35 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sat, 16 Sep 2017 12:06:06 -0700 Subject: [PATCH 3/6] Collapsed commits formatting fixes --- dvc/graph/commit.py | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/dvc/graph/commit.py b/dvc/graph/commit.py index 6fd74c146..e22d2d56a 100644 --- a/dvc/graph/commit.py +++ b/dvc/graph/commit.py @@ -16,7 +16,7 @@ class Commit(object): self._target_metric = target_metric self._target_metric_delta = None self._branch_tips = [] if branch_tips is None else branch_tips - self._added_commits_pairs = [] + self._collapsed_commits = [] self._is_collapsed = False @@ -35,7 +35,7 @@ class Commit(object): self._parent_hashes.remove(hash) def add_collapsed_commit(self, commit): - self._added_commits_pairs.append((commit.hash, commit._comment)) + self._collapsed_commits.append(commit) @property def text(self): @@ -43,28 +43,37 @@ class Commit(object): if self._branch_tips: branch_text = 'Branch tips: {}\n'.format(', '.join(self._branch_tips)) - return self._text_metrics_line() + branch_text + self._text_comment + self._text_added_comments + # text = self._text_comment(self.hash, self._comment, self._is_collapsed) + text = '' + return self._text_metrics_line() + branch_text + text + self._text_added_comments def _text_hash(self): return 'Commit: ' + self.hash - @property - def _text_comment(self): - text = self.COLLAPSED_TEXT if self._is_collapsed else self._limit_text(self._comment) - return '{}: {}'.format(self.hash, text) + # @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 _limit_text(self, comment): - if len(comment) < self.TEXT_LIMIT: - return self._comment + @staticmethod + def _limit_text(comment): + if len(comment) < Commit.TEXT_LIMIT: + return comment else: - return comment[:self.TEXT_LIMIT-3] + '...' + return comment[:Commit.TEXT_LIMIT-3] + '...' @property def _text_added_comments(self): - res = '' - for hash, commit in self._added_commits_pairs: - res += '\n{}: {}'.format(hash, self._limit_text(commit)) - return res + res = [self._text_comment(self.hash, self._comment, self._is_collapsed)] + + for commit in self._collapsed_commits: + text = self._text_comment(commit.hash, commit._comment, commit.is_repro) + res.append(text) + + max_len = max(map(lambda x: len(x), res)) + res_extended_len = map(lambda x: x.ljust(max_len), res) + return '\n'.join(res_extended_len) def _text_metrics_line(self): result = '' From 563ad0f5131bd2579944eb77b1fbcc4eb79c3af7 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sat, 16 Sep 2017 22:11:32 -0700 Subject: [PATCH 4/6] Final changes and unit-tests fixes --- dvc/cli.py | 14 ++++---- dvc/command/show_workflow.py | 2 +- dvc/graph/commit.py | 34 ++++++++++---------- dvc/graph/workflow.py | 49 +++++++++++----------------- tests/test_workflow.py | 62 +++++++++++++++++++++++++++--------- 5 files changed, 91 insertions(+), 70 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index 106e840ec..8f2215797 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -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) diff --git a/dvc/command/show_workflow.py b/dvc/command/show_workflow.py index 8008939a4..37b6bdc2e 100644 --- a/dvc/command/show_workflow.py +++ b/dvc/command/show_workflow.py @@ -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 diff --git a/dvc/graph/commit.py b/dvc/graph/commit.py index e22d2d56a..754c3bad3 100644 --- a/dvc/graph/commit.py +++ b/dvc/graph/commit.py @@ -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' diff --git a/dvc/graph/workflow.py b/dvc/graph/workflow.py index 4b0bb1914..2fd6d4ab2 100644 --- a/dvc/graph/workflow.py +++ b/dvc/graph/workflow.py @@ -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: diff --git a/tests/test_workflow.py b/tests/test_workflow.py index a9a6f0aa5..db0997745 100644 --- a/tests/test_workflow.py +++ b/tests/test_workflow.py @@ -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 \ No newline at end of file + 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 From d77a63d4275ae67c5af0ce9338fcd1fe1e935b77 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sat, 16 Sep 2017 22:22:39 -0700 Subject: [PATCH 5/6] Refactoring: code climate imporvements --- dvc/graph/workflow.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/dvc/graph/workflow.py b/dvc/graph/workflow.py index 2fd6d4ab2..7f839b602 100644 --- a/dvc/graph/workflow.py +++ b/dvc/graph/workflow.py @@ -104,21 +104,11 @@ class Workflow(object): return [] return [self._commits[h] for h in self._edges[hash]] - def parent_commits(self, hash): - if hash not in self._back_edges: - return [] - return [self._commits[h] for h in self._back_edges[hash]] - def build_graph(self, show_dvc_commits, show_all_commits): + self.modify_workflow(show_all_commits, show_dvc_commits) + g = nx.DiGraph(name='DVC Workflow', directed=False) - self.derive_target_metric_deltas() - - 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] @@ -140,6 +130,13 @@ class Workflow(object): A.draw(fname, format='jpeg', prog='dot') pass + def modify_workflow(self, show_all_commits, show_dvc_commits): + self.derive_target_metric_deltas() + if not show_all_commits: + self.collapse_commits(CollapseDvcReproCommitsStrategy(self)) + if not show_dvc_commits: + self.collapse_commits(CollapseNotMeticsCommitsStrategy(self)) + @staticmethod def node_color(commit): if commit.target_metric_delta is not None: @@ -166,6 +163,9 @@ class Workflow(object): if self._remove_or_collapse(commit, strategy): hashes_to_remove.append(commit.hash) + self.remove_hashes(hashes_to_remove) + + def remove_hashes(self, hashes_to_remove): for hash in hashes_to_remove: del self._commits[hash] if hash in self._edges: From 82b51efe1902e55056eca2503d9b4d08d9ff7913 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sat, 16 Sep 2017 22:44:56 -0700 Subject: [PATCH 6/6] Add xmltodict to requirements for functests --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 8754dcc29..d1b30b269 100644 --- a/requirements.txt +++ b/requirements.txt @@ -88,3 +88,4 @@ colorama==0.3.9 google-compute-engine==2.4.1 configobj==5.0.6 networkx==1.11 +xmltodict==0.11.0