From 95f39de424f353627b0cd0ac877cfb0d7679262d Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 13 Sep 2017 00:58:49 -0700 Subject: [PATCH] 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)