Repository: iterative/dvc

Issue:
diff: detect renames
File renaming currently shows up in DVC DIFF <another_git_branch> as:

Added: (new filename after renaming)
Deleted: (old filename)

Would it be possible to group this information into a new category "Renamed" so that the overview is a bit clearer?

P.s.: If somebody could point me into the right direction, would like to create a pull request myself.


Discussion:
More context: https://discuss.dvc.org/t/merging-of-files-in-dvc-tracked-directories/599/5

Patch:
diff --git a/dvc/command/diff.py b/dvc/command/diff.py
--- a/dvc/command/diff.py
+++ b/dvc/command/diff.py
@@ -22,7 +22,7 @@ def _show_md(diff, show_hash=False, hide_missing=False):
 
     header = ["Status", "Hash", "Path"] if show_hash else ["Status", "Path"]
     rows = []
-    statuses = ["added", "deleted", "modified"]
+    statuses = ["added", "deleted", "renamed", "modified"]
     if not hide_missing:
         statuses.append("not in cache")
     for status in statuses:
@@ -31,6 +31,8 @@ def _show_md(diff, show_hash=False, hide_missing=False):
             continue
         for entry in entries:
             path = entry["path"]
+            if isinstance(path, dict):
+                path = f"{path['old']} -> {path['new']}"
             if show_hash:
                 check_sum = _digest(entry.get("hash", ""))
                 rows.append([status, check_sum, path])
@@ -72,13 +74,14 @@ def _format(diff, hide_missing=False):
             "added": colorama.Fore.GREEN,
             "modified": colorama.Fore.YELLOW,
             "deleted": colorama.Fore.RED,
+            "renamed": colorama.Fore.GREEN,
             "not in cache": colorama.Fore.YELLOW,
         }
 
         summary = {}
         groups = []
 
-        states = ["added", "deleted", "modified"]
+        states = ["added", "deleted", "renamed", "modified"]
         if not hide_missing:
             states.append("not in cache")
         for state in states:
@@ -92,6 +95,8 @@ def _format(diff, hide_missing=False):
 
             for entry in entries:
                 path = entry["path"]
+                if isinstance(path, dict):
+                    path = f"{path['old']} -> {path['new']}"
                 checksum = entry.get("hash")
                 summary[state] += 1 if not path.endswith(os.sep) else 0
                 content.append(
@@ -99,7 +104,7 @@ def _format(diff, hide_missing=False):
                         space="    ",
                         checksum=_digest(checksum) if checksum else "",
                         separator="  " if checksum else "",
-                        path=entry["path"],
+                        path=path,
                     )
                 )
 
@@ -117,7 +122,7 @@ def _format(diff, hide_missing=False):
 
         fmt = (
             "files summary: {added} added, {deleted} deleted,"
-            " {modified} modified"
+            " {renamed} renamed, {modified} modified"
         )
         if not hide_missing:
             fmt += ", {not in cache} not in cache"
@@ -138,7 +143,12 @@ def run(self):
                 del diff["not in cache"]
 
             for key, entries in diff.items():
-                entries = sorted(entries, key=lambda entry: entry["path"])
+                entries = sorted(
+                    entries,
+                    key=lambda entry: entry["path"]["old"]
+                    if isinstance(entry["path"], dict)
+                    else entry["path"],
+                )
                 if not show_hash:
                     for entry in entries:
                         del entry["hash"]
diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py
--- a/dvc/repo/diff.py
+++ b/dvc/repo/diff.py
@@ -65,6 +65,17 @@ def diff(self, a_rev="HEAD", b_rev=None, targets=None):
     deleted = sorted(deleted_or_missing - set(missing))
     modified = sorted(set(old) & set(new))
 
+    # Cases when file was changed and renamed are resulted
+    # in having deleted and added record
+    # To cover such cases we need to change hashing function
+    # to produce rolling/chunking hash
+
+    renamed = _calculate_renamed(new, old, added, deleted)
+
+    for renamed_item in renamed:
+        added.remove(renamed_item["path"]["new"])
+        deleted.remove(renamed_item["path"]["old"])
+
     ret = {
         "added": [{"path": path, "hash": new[path]} for path in added],
         "deleted": [{"path": path, "hash": old[path]} for path in deleted],
@@ -73,6 +84,7 @@ def diff(self, a_rev="HEAD", b_rev=None, targets=None):
             for path in modified
             if old[path] != new[path]
         ],
+        "renamed": renamed,
         "not in cache": [
             {"path": path, "hash": old[path]} for path in missing
         ],
@@ -171,3 +183,34 @@ def _targets_to_path_infos(repo_tree, targets):
             missing.append(target)
 
     return path_infos, missing
+
+
+def _calculate_renamed(new, old, added, deleted):
+    old_inverted = {}
+    # It is needed to be dict of lists to cover cases
+    # when repo has paths with same hash
+    for path, path_hash in old.items():
+        bucket = old_inverted.get(path_hash, None)
+        if bucket is None:
+            old_inverted[path_hash] = [path]
+        else:
+            bucket.append(path)
+
+    renamed = []
+    for path in added:
+        path_hash = new[path]
+        old_paths = old_inverted.get(path_hash, None)
+        if not old_paths:
+            continue
+        old_path = None
+        for tmp_old_path in old_paths:
+            if tmp_old_path in deleted:
+                old_path = tmp_old_path
+                break
+        if not old_path:
+            continue
+        renamed.append(
+            {"path": {"old": old_path, "new": path}, "hash": path_hash}
+        )
+
+    return renamed


Test patch:
diff --git a/tests/func/params/test_diff.py b/tests/func/params/test_diff.py
--- a/tests/func/params/test_diff.py
+++ b/tests/func/params/test_diff.py
@@ -1,3 +1,7 @@
+import os
+
+import pytest
+
 from dvc.utils import relpath
 from dvc.utils.serialize import dump_yaml
 
@@ -14,6 +18,132 @@ def test_diff_no_changes(tmp_dir, scm, dvc):
     assert dvc.params.diff() == {}
 
 
+@pytest.mark.parametrize("commit_last", [True, False])
+def test_diff_add_similar_files(tmp_dir, scm, dvc, commit_last):
+    if commit_last:
+        last_commit_msg = "commit #2"
+        a_rev = "HEAD~1"
+    else:
+        last_commit_msg = None
+        a_rev = "HEAD"
+
+    tmp_dir.dvc_gen(
+        {"dir": {"file": "text1", "subdir": {"file2": "text2"}}},
+        commit="commit #1",
+    )
+    tmp_dir.dvc_gen(
+        {"dir2": {"file": "text1", "subdir": {"file2": "text2"}}},
+        commit=last_commit_msg,
+    )
+    assert dvc.diff(a_rev) == {
+        "added": [
+            {
+                "path": os.path.join("dir2", ""),
+                "hash": "cb58ee07cb01044db229e4d6121a0dfc.dir",
+            },
+            {
+                "path": os.path.join("dir2", "file"),
+                "hash": "cef7ccd89dacf1ced6f5ec91d759953f",
+            },
+            {
+                "path": os.path.join("dir2", "subdir", "file2"),
+                "hash": "fe6123a759017e4a2af4a2d19961ed71",
+            },
+        ],
+        "deleted": [],
+        "modified": [],
+        "renamed": [],
+        "not in cache": [],
+    }
+
+
+@pytest.mark.parametrize("commit_last", [True, False])
+def test_diff_rename_folder(tmp_dir, scm, dvc, commit_last):
+    if commit_last:
+        last_commit_msg = "commit #2"
+        a_rev = "HEAD~1"
+    else:
+        last_commit_msg = None
+        a_rev = "HEAD"
+
+    tmp_dir.dvc_gen(
+        {"dir": {"file": "text1", "subdir": {"file2": "text2"}}},
+        commit="commit #1",
+    )
+    (tmp_dir / "dir").replace(tmp_dir / "dir2")
+    tmp_dir.dvc_add("dir2", commit=last_commit_msg)
+    assert dvc.diff(a_rev) == {
+        "added": [],
+        "deleted": [],
+        "modified": [],
+        "renamed": [
+            {
+                "path": {
+                    "old": os.path.join("dir", ""),
+                    "new": os.path.join("dir2", ""),
+                },
+                "hash": "cb58ee07cb01044db229e4d6121a0dfc.dir",
+            },
+            {
+                "path": {
+                    "old": os.path.join("dir", "file"),
+                    "new": os.path.join("dir2", "file"),
+                },
+                "hash": "cef7ccd89dacf1ced6f5ec91d759953f",
+            },
+            {
+                "path": {
+                    "old": os.path.join("dir", "subdir", "file2"),
+                    "new": os.path.join("dir2", "subdir", "file2"),
+                },
+                "hash": "fe6123a759017e4a2af4a2d19961ed71",
+            },
+        ],
+        "not in cache": [],
+    }
+
+
+@pytest.mark.parametrize("commit_last", [True, False])
+def test_diff_rename_file(tmp_dir, scm, dvc, commit_last):
+    if commit_last:
+        last_commit_msg = "commit #2"
+        a_rev = "HEAD~1"
+    else:
+        last_commit_msg = None
+        a_rev = "HEAD"
+
+    paths = tmp_dir.gen(
+        {"dir": {"file": "text1", "subdir": {"file2": "text2"}}}
+    )
+    tmp_dir.dvc_add(paths, commit="commit #1")
+    (tmp_dir / "dir" / "file").replace(tmp_dir / "dir" / "subdir" / "file3")
+
+    tmp_dir.dvc_add(paths, commit=last_commit_msg)
+    assert dvc.diff(a_rev) == {
+        "added": [],
+        "deleted": [],
+        "modified": [
+            {
+                "path": os.path.join("dir", ""),
+                "hash": {
+                    "old": "cb58ee07cb01044db229e4d6121a0dfc.dir",
+                    "new": "a4ac9c339aacc60b6a3152e362c319c8.dir",
+                },
+            }
+        ],
+        "renamed": [
+            {
+                "path": {
+                    "old": os.path.join("dir", "file"),
+                    "new": os.path.join("dir", "subdir", "file3"),
+                },
+                "hash": "cef7ccd89dacf1ced6f5ec91d759953f",
+            }
+        ],
+        "not in cache": [],
+    }
+
+
 def test_diff(tmp_dir, scm, dvc):
     tmp_dir.gen("params.yaml", "foo: bar")
     dvc.run(cmd="echo params.yaml", params=["foo"], single_stage=True)
diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py
--- a/tests/func/test_diff.py
+++ b/tests/func/test_diff.py
@@ -29,6 +29,7 @@ def test_added(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -57,6 +58,7 @@ def test_no_cache_entry(tmp_dir, scm, dvc):
             }
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -72,6 +74,7 @@ def test_deleted(tmp_dir, scm, dvc, delete_data):
         "deleted": [{"path": "file", "hash": digest("text")}],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -89,6 +92,7 @@ def test_modified(tmp_dir, scm, dvc):
             }
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -106,6 +110,7 @@ def test_refs(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [{"path": "file", "hash": {"old": HEAD_1, "new": HEAD}}],
         "not in cache": [],
+        "renamed": [],
     }
 
     assert dvc.diff("HEAD~2", "HEAD~1") == {
@@ -113,6 +118,7 @@ def test_refs(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [{"path": "file", "hash": {"old": HEAD_2, "new": HEAD_1}}],
         "not in cache": [],
+        "renamed": [],
     }
 
     with pytest.raises(DvcException, match=r"unknown Git revision 'missing'"):
@@ -144,6 +150,7 @@ def test_directories(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
     assert dvc.diff(":/directory", ":/modify") == {
@@ -163,6 +170,7 @@ def test_directories(tmp_dir, scm, dvc):
             },
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
     assert dvc.diff(":/modify", ":/delete") == {
@@ -180,6 +188,7 @@ def test_directories(tmp_dir, scm, dvc):
             }
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -208,6 +217,7 @@ def test_diff_no_cache(tmp_dir, scm, dvc):
     diff = dvc.diff()
     assert diff["added"] == []
     assert diff["deleted"] == []
+    assert diff["renamed"] == []
     assert diff["modified"] == []
     assert diff["not in cache"] == [
         {
@@ -250,6 +260,7 @@ def test_diff_dirty(tmp_dir, scm, dvc):
             }
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -300,6 +311,7 @@ def test_targets_single_file(tmp_dir, scm, dvc):
             }
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -317,6 +329,7 @@ def test_targets_single_dir(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
     assert dvc.diff(targets=["dir"]) == expected_result
@@ -331,6 +344,7 @@ def test_targets_single_file_in_dir(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -347,6 +361,7 @@ def test_targets_two_files_in_dir(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -369,6 +384,7 @@ def test_targets_file_and_dir(tmp_dir, scm, dvc):
             }
         ],
         "not in cache": [],
+        "renamed": [],
     }
 
 
@@ -385,6 +401,7 @@ def test_targets_single_dir_with_file(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
 
     assert dvc.diff(targets=["dir_with"]) == expected_result
@@ -404,4 +421,5 @@ def test_targets_single_file_in_dir_with_file(tmp_dir, scm, dvc):
         "deleted": [],
         "modified": [],
         "not in cache": [],
+        "renamed": [],
     }
diff --git a/tests/unit/command/test_diff.py b/tests/unit/command/test_diff.py
--- a/tests/unit/command/test_diff.py
+++ b/tests/unit/command/test_diff.py
@@ -28,6 +28,15 @@ def test_default(mocker, caplog):
         "added": [{"path": "file", "hash": "00000000"}],
         "deleted": [],
         "modified": [],
+        "renamed": [
+            {
+                "path": {
+                    "old": os.path.join("data", "file_old"),
+                    "new": os.path.join("data", "file_new"),
+                },
+                "hash": "11111111",
+            }
+        ],
         "not in cache": [],
     }
     mocker.patch("dvc.repo.Repo.diff", return_value=diff)
@@ -37,8 +46,12 @@ def test_default(mocker, caplog):
         "Added:\n"
         "    file\n"
         "\n"
-        "files summary: 1 added, 0 deleted, 0 modified, 0 not in cache"
-    ) in caplog.text
+        "Renamed:\n"
+        "    data{sep}file_old -> data{sep}file_new\n"
+        "\n"
+        "files summary: 1 added, 0 deleted, "
+        "1 renamed, 0 modified, 0 not in cache"
+    ).format(sep=os.path.sep) in caplog.text
 
 
 def test_show_hash(mocker, caplog):
@@ -55,6 +68,15 @@ def test_show_hash(mocker, caplog):
             {"path": "file2", "hash": {"old": "AAAAAAAA", "new": "BBBBBBBB"}},
             {"path": "file1", "hash": {"old": "CCCCCCCC", "new": "DDDDDDDD"}},
         ],
+        "renamed": [
+            {
+                "path": {
+                    "old": os.path.join("data", "file_old"),
+                    "new": os.path.join("data", "file_new"),
+                },
+                "hash": "11111111",
+            }
+        ],
         "not in cache": [],
     }
     mocker.patch("dvc.repo.Repo.diff", return_value=diff)
@@ -65,11 +87,19 @@ def test_show_hash(mocker, caplog):
         "    00000000  " + os.path.join("data", "bar") + "\n"
         "    11111111  " + os.path.join("data", "foo") + "\n"
         "\n"
+        "Renamed:\n"
+        "    11111111  "
+        + os.path.join("data", "file_old")
+        + " -> "
+        + os.path.join("data", "file_new")
+        + "\n"
+        "\n"
         "Modified:\n"
         "    CCCCCCCC..DDDDDDDD  file1\n"
         "    AAAAAAAA..BBBBBBBB  file2\n"
         "\n"
-        "files summary: 0 added, 2 deleted, 2 modified, 0 not in cache"
+        "files summary: 0 added, 2 deleted, "
+        "1 renamed, 2 modified, 0 not in cache"
     ) in caplog.text
 
 
@@ -106,6 +136,12 @@ def test_show_json_and_hash(mocker, caplog):
         ],
         "deleted": [],
         "modified": [],
+        "renamed": [
+            {
+                "path": {"old": "file_old", "new": "file_new"},
+                "hash": "11111111",
+            }
+        ],
         "not in cache": [],
     }
     mocker.patch("dvc.repo.Repo.diff", return_value=diff)
@@ -117,6 +153,10 @@ def test_show_json_and_hash(mocker, caplog):
     )
     assert '"deleted": []' in caplog.text
     assert '"modified": []' in caplog.text
+    assert (
+        '"renamed": [{"path": {"old": "file_old", '
+        '"new": "file_new"}, "hash": "11111111"}]' in caplog.text
+    )
     assert '"not in cache": []' in caplog.text
 
 
@@ -130,6 +170,12 @@ def test_show_json_hide_missing(mocker, caplog):
         ],
         "deleted": [],
         "modified": [],
+        "renamed": [
+            {
+                "path": {"old": "file_old", "new": "file_new"},
+                "hash": "11111111",
+            }
+        ],
         "not in cache": [],
     }
     mocker.patch("dvc.repo.Repo.diff", return_value=diff)
@@ -137,6 +183,10 @@ def test_show_json_hide_missing(mocker, caplog):
     assert 0 == cmd.run()
     assert '"added": [{"path": "file1"}, {"path": "file2"}]' in caplog.text
     assert '"deleted": []' in caplog.text
+    assert (
+        '"renamed": [{"path": {"old": "file_old", "new": "file_new"}'
+        in caplog.text
+    )
     assert '"modified": []' in caplog.text
     assert '"not in cache": []' not in caplog.text
 
@@ -193,18 +243,20 @@ def test_show_md():
         ],
         "modified": [{"path": "file"}],
         "added": [{"path": "file"}],
+        "renamed": [{"path": {"old": "file_old", "new": "file_new"}}],
         "not in cache": [{"path": "file2"}],
     }
     assert _show_md(diff) == (
-        "| Status       | Path     |\n"
-        "|--------------|----------|\n"
-        "| added        | file     |\n"
-        "| deleted      | zoo      |\n"
-        "| deleted      | data{sep}    |\n"
-        "| deleted      | data{sep}foo |\n"
-        "| deleted      | data{sep}bar |\n"
-        "| modified     | file     |\n"
-        "| not in cache | file2    |\n"
+        "| Status       | Path                 |\n"
+        "|--------------|----------------------|\n"
+        "| added        | file                 |\n"
+        "| deleted      | zoo                  |\n"
+        "| deleted      | data{sep}                |\n"
+        "| deleted      | data{sep}foo             |\n"
+        "| deleted      | data{sep}bar             |\n"
+        "| renamed      | file_old -> file_new |\n"
+        "| modified     | file                 |\n"
+        "| not in cache | file2                |\n"
     ).format(sep=os.path.sep)
 
 
@@ -220,18 +272,25 @@ def test_show_md_with_hash():
             {"path": "file", "hash": {"old": "AAAAAAAA", "new": "BBBBBBBB"}}
         ],
         "added": [{"path": "file", "hash": "00000000"}],
+        "renamed": [
+            {
+                "path": {"old": "file_old", "new": "file_new"},
+                "hash": "11111111",
+            }
+        ],
         "not in cache": [{"path": "file2", "hash": "12345678"}],
     }
     assert _show_md(diff, show_hash=True) == (
-        "| Status       | Hash               | Path     |\n"
-        "|--------------|--------------------|----------|\n"
-        "| added        | 00000000           | file     |\n"
-        "| deleted      | 22222              | zoo      |\n"
-        "| deleted      | XXXXXXXX           | data{sep}    |\n"
-        "| deleted      | 11111111           | data{sep}foo |\n"
-        "| deleted      | 00000000           | data{sep}bar |\n"
-        "| modified     | AAAAAAAA..BBBBBBBB | file     |\n"
-        "| not in cache | 12345678           | file2    |\n"
+        "| Status       | Hash               | Path                 |\n"
+        "|--------------|--------------------|----------------------|\n"
+        "| added        | 00000000           | file                 |\n"
+        "| deleted      | 22222              | zoo                  |\n"
+        "| deleted      | XXXXXXXX           | data{sep}                |\n"
+        "| deleted      | 11111111           | data{sep}foo             |\n"
+        "| deleted      | 00000000           | data{sep}bar             |\n"
+        "| renamed      | 11111111           | file_old -> file_new |\n"
+        "| modified     | AAAAAAAA..BBBBBBBB | file                 |\n"
+        "| not in cache | 12345678           | file2                |\n"
     ).format(sep=os.path.sep)
 
 
@@ -245,17 +304,19 @@ def test_show_md_hide_missing():
         ],
         "modified": [{"path": "file"}],
         "added": [{"path": "file"}],
+        "renamed": [{"path": {"old": "file_old", "new": "file_new"}}],
         "not in cache": [{"path": "file2"}],
     }
     assert _show_md(diff, hide_missing=True) == (
-        "| Status   | Path     |\n"
-        "|----------|----------|\n"
-        "| added    | file     |\n"
-        "| deleted  | zoo      |\n"
-        "| deleted  | data{sep}    |\n"
-        "| deleted  | data{sep}foo |\n"
-        "| deleted  | data{sep}bar |\n"
-        "| modified | file     |\n"
+        "| Status   | Path                 |\n"
+        "|----------|----------------------|\n"
+        "| added    | file                 |\n"
+        "| deleted  | zoo                  |\n"
+        "| deleted  | data{sep}                |\n"
+        "| deleted  | data{sep}foo             |\n"
+        "| deleted  | data{sep}bar             |\n"
+        "| renamed  | file_old -> file_new |\n"
+        "| modified | file                 |\n"
     ).format(sep=os.path.sep)
 
 
@@ -266,6 +327,12 @@ def test_hide_missing(mocker, caplog):
         "added": [{"path": "file", "hash": "00000000"}],
         "deleted": [],
         "modified": [],
+        "renamed": [
+            {
+                "path": {"old": "file_old", "new": "file_new"},
+                "hash": "11111111",
+            }
+        ],
         "not in cache": [],
     }
     mocker.patch("dvc.repo.Repo.diff", return_value=diff)
@@ -275,6 +342,9 @@ def test_hide_missing(mocker, caplog):
         "Added:\n"
         "    file\n"
         "\n"
-        "files summary: 1 added, 0 deleted, 0 modified"
+        "Renamed:\n"
+        "    file_old -> file_new\n"
+        "\n"
+        "files summary: 1 added, 0 deleted, 1 renamed, 0 modified"
     ) in caplog.text
     assert "not in cache" not in caplog.text