From 23e4d376c9179388d6a8c9f92f34e01c982f4c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Tue, 20 Dec 2022 11:41:34 +0545 Subject: [PATCH] index: remove unused APIs These are being used by some users, but removing it for easier refactoring. We are not using it ourselves. --- dvc/repo/index.py | 135 ++-------------------------------- dvc/repo/stage.py | 4 +- tests/func/test_repo_index.py | 120 +----------------------------- 3 files changed, 11 insertions(+), 248 deletions(-) diff --git a/dvc/repo/index.py b/dvc/repo/index.py index 83e36da9b..c988b1f0b 100644 --- a/dvc/repo/index.py +++ b/dvc/repo/index.py @@ -1,4 +1,3 @@ -from contextlib import suppress from typing import ( TYPE_CHECKING, Any, @@ -14,9 +13,7 @@ from typing import ( Union, ) -from funcy import cached_property, nullcontext - -from dvc.utils import dict_md5 +from funcy import cached_property if TYPE_CHECKING: from networkx import DiGraph @@ -25,9 +22,10 @@ if TYPE_CHECKING: from dvc.dependency import Dependency, ParamsDependency from dvc.fs import FileSystem from dvc.output import Output + from dvc.repo import Repo from dvc.repo.stage import StageInfo, StageLoad from dvc.stage import Stage - from dvc.types import StrPath, TargetType + from dvc.types import TargetType from dvc_data.hashfile.hash_info import HashInfo from dvc_data.index import DataIndex, DataIndexKey, DataIndexView from dvc_objects.db import ObjectDB @@ -136,38 +134,6 @@ class Index: rev = self.repo.get_rev()[:7] return f"Index({self.repo}, fs@{rev})" - def __len__(self) -> int: - return len(self.stages) - - def __contains__(self, stage: "Stage") -> bool: - # as we are keeping stages inside a list, it might be slower. - return stage in self.stages - - def __iter__(self) -> Iterator["Stage"]: - yield from self.stages - - def __getitem__(self, item: str) -> "Stage": - """Get a stage by its addressing attribute.""" - for stage in self: - if stage.addressing == item: - return stage - raise KeyError(f"{item} - available stages are {self.stages}") - - def filter(self, filter_fn: Callable[["Stage"], bool]) -> "Index": - stages_it = filter(filter_fn, self) - return Index(self.repo, self.fs, stages=list(stages_it)) - - def slice(self, path: "StrPath") -> "Index": - from dvc.utils import relpath - from dvc.utils.fs import path_isin - - target_path = relpath(path, self.repo.root_dir) - - def is_stage_inside_path(stage: "Stage") -> bool: - return path_isin(stage.path_in_repo, target_path) - - return self.filter(is_stage_inside_path) - @staticmethod def _hash_targets( targets: Iterable[Optional[str]], @@ -236,7 +202,7 @@ class Index: @property def outs(self) -> Iterator["Output"]: - for stage in self: + for stage in self.stages: yield from stage.outs @property @@ -259,7 +225,7 @@ class Index: @property def deps(self) -> Iterator["Dependency"]: - for stage in self: + for stage in self.stages: yield from stage.deps @property @@ -353,56 +319,10 @@ class Index: def add(self, stage: "Stage") -> "Index": return self.update([stage]) - def remove( - self, stage: "Stage", ignore_not_existing: bool = False - ) -> "Index": - stages = self._discard_stage( - stage, ignore_not_existing=ignore_not_existing - ) - return Index(self.repo, self.fs, stages=stages) - - def discard(self, stage: "Stage") -> "Index": - return self.remove(stage, ignore_not_existing=True) - - def difference(self, stages: Iterable["Stage"]) -> "Index": - stages_set = set(self.stages) - set(stages) - return Index(self.repo, self.fs, stages=list(stages_set)) - - def _discard_stage( - self, stage: "Stage", ignore_not_existing: bool = False - ) -> List["Stage"]: - stages = self.stages[:] - ctx = suppress(ValueError) if ignore_not_existing else nullcontext() - with ctx: - stages.remove(stage) - return stages - def check_graph(self) -> None: if not getattr(self.repo, "_skip_graph_checks", False): self.graph # pylint: disable=pointless-statement - def dumpd(self) -> Dict[str, Dict]: - def dump(stage: "Stage"): - key = stage.path_in_repo - try: - key += ":" + stage.name # type: ignore[attr-defined] - except AttributeError: - pass - return key, stage.dumpd() - - return dict(dump(stage) for stage in self) - - @cached_property - def identifier(self) -> str: - """Unique identifier for the index. - - We can use this to optimize and skip opening some indices - eg: on push/pull/fetch/gc --all-commits. - - Currently, it is unique to the platform (windows vs posix). - """ - return dict_md5(self.dumpd()) - class _DataPrefixes(NamedTuple): explicit: Set["DataIndexKey"] @@ -425,30 +345,13 @@ class IndexView: self._stages = list({stage for stage, _ in stage_infos}) self._outs_filter = outs_filter - def __len__(self) -> int: - return len(self._stages) - - def __contains__(self, stage: "Stage") -> bool: - # as we are keeping stages inside a list, it might be slower. - return stage in self._stages - - def __iter__(self) -> Iterator["Stage"]: - yield from self._stages - - def __getitem__(self, item: str) -> "Stage": - """Get a stage by its addressing attribute.""" - for stage in self: - if stage.addressing == item: - return stage - raise KeyError(f"{item} - available stages are {self.stages}") - @property def stages(self): return self._stages @property def deps(self) -> Iterator["Dependency"]: - for stage in self: + for stage in self.stages: yield from stage.deps @property @@ -505,29 +408,3 @@ class IndexView: else: data[workspace] = DataIndex() return data - - -if __name__ == "__main__": - import logging - - from funcy import log_durations - - from dvc.logger import setup - from dvc.repo import Repo - - setup(level=logging.TRACE) # type: ignore[attr-defined] - - repo = Repo() - index = Index(repo, repo.fs) - print(index) - with log_durations(print, "collecting stages"): - # pylint: disable=pointless-statement - print("no of stages", len(index.stages)) - with log_durations(print, "building graph"): - index.graph # pylint: disable=pointless-statement - with log_durations(print, "calculating hash"): - print(index.identifier) - with log_durations(print, "updating"): - index2 = index.update(index.stages) - with log_durations(print, "calculating hash"): - print(index2.identifier) diff --git a/dvc/repo/stage.py b/dvc/repo/stage.py index f459b6474..b1f3008cb 100644 --- a/dvc/repo/stage.py +++ b/dvc/repo/stage.py @@ -369,7 +369,7 @@ class StageLoad: glob: Use `target` as a pattern to match stages in a file. """ if not target: - return list(graph) if graph else list(self.repo.index) + return list(graph) if graph else self.repo.index.stages if recursive and self.fs.isdir(target): from dvc.repo.graph import collect_inside_path @@ -410,7 +410,7 @@ class StageLoad: (see `collect()` for other arguments) """ if not target: - return [StageInfo(stage) for stage in self.repo.index] + return [StageInfo(stage) for stage in self.repo.index.stages] target = as_posix(target) diff --git a/tests/func/test_repo_index.py b/tests/func/test_repo_index.py index c3dbec448..f5e965fd3 100644 --- a/tests/func/test_repo_index.py +++ b/tests/func/test_repo_index.py @@ -1,12 +1,10 @@ -import os from itertools import chain import pytest from pygtrie import Trie from dvc.repo.index import Index -from dvc.stage import PipelineStage, Stage -from dvc.utils import relpath +from dvc.stage import Stage def test_index(tmp_dir, scm, dvc, run_copy): @@ -17,15 +15,11 @@ def test_index(tmp_dir, scm, dvc, run_copy): index = Index(dvc) assert index.fs == dvc.fs - assert len(index) == len(index.stages) == 2 - assert set(index.stages) == set(index) == {stage1, stage2} - assert stage1 in index - assert stage2 in index + assert set(index.stages) == {stage1, stage2} assert index.outs_graph assert index.graph assert isinstance(index.outs_trie, Trie) - assert index.identifier index.check_graph() @@ -42,41 +36,6 @@ def test_repr(tmp_dir, scm, dvc): assert repr(Index(dvc)) == f"Index({dvc}, fs@{rev[:7]})" -def test_filter_index(tmp_dir, dvc, run_copy): - tmp_dir.dvc_gen("foo", "foo") - stage2 = run_copy("foo", "bar", name="copy-foo-bar") - - def filter_pipeline(stage): - return bool(stage.cmd) - - filtered_index = Index(dvc).filter(filter_pipeline) - assert list(filtered_index) == [stage2] - - -def test_slice_index(tmp_dir, dvc): - tmp_dir.gen({"dir1": {"foo": "foo"}, "dir2": {"bar": "bar"}}) - with (tmp_dir / "dir1").chdir(): - (stage1,) = dvc.add("foo") - with (tmp_dir / "dir2").chdir(): - (stage2,) = dvc.add("bar") - - index = Index(dvc) - - sliced = index.slice("dir1") - assert set(sliced) == {stage1} - assert sliced.stages is not index.stages # sanity check - - sliced = index.slice(tmp_dir / "dir1") - assert set(sliced) == {stage1} - - sliced = index.slice("dir2") - assert set(sliced) == {stage2} - - with (tmp_dir / "dir1").chdir(): - sliced = index.slice(relpath(tmp_dir / "dir2")) - assert set(sliced) == {stage2} - - def outputs_equal(actual, expected): actual, expected = list(actual), list(expected) @@ -147,7 +106,7 @@ def test_add_update(dvc): dup_stage2 = Stage(dvc, path="path2") dup_index = index.update([dup_stage1, dup_stage2]) assert not index.stages - assert len(new_index) == 1 + assert len(new_index.stages) == 1 assert new_index.stages[0] is new_stage assert set(map(id, dup_index.stages)) == {id(dup_stage1), id(dup_stage2)} @@ -165,67 +124,6 @@ def assert_index_equal(first, second, strict=True, ordered=True): ), "Index is not strictly equal" -def test_discard_remove(dvc): - stage = Stage(dvc, path="path1") - index = Index(dvc, stages=[stage]) - - assert list(index.discard(Stage(dvc, "path2"))) == list(index) - new_index = index.discard(stage) - assert len(new_index) == 0 - - with pytest.raises(ValueError): - index.remove(Stage(dvc, "path2")) - assert index.stages == [stage] - assert list(index.remove(stage)) == [] - - -def test_difference(dvc): - stages = [Stage(dvc, path=f"path{i}") for i in range(10)] - index = Index(dvc, stages=stages) - - new_index = index.difference([*stages[:5], Stage(dvc, path="path100")]) - assert index.stages == stages - assert set(new_index) == set(stages[5:]) - - -def test_dumpd(dvc): - stages = [ - PipelineStage(dvc, "dvc.yaml", name="stage1"), - Stage(dvc, "path"), - ] - index = Index(dvc, stages=stages) - assert index.dumpd() == {"dvc.yaml:stage1": {}, "path": {}} - assert index.identifier == "d43da84e9001540c26abf2bf4541c275" - - -def test_unique_identifier(tmp_dir, dvc, scm, run_copy): - dvc.scm_context.autostage = True - tmp_dir.dvc_gen("foo", "foo") - run_copy("foo", "bar", name="copy-foo-bar") - - revs = [] - n_commits = 5 - for i in range(n_commits): - # create a few empty commits - scm.commit(f"commit {i}") - revs.append(scm.get_rev()) - assert len(set(revs)) == n_commits # the commit revs should be unique - - ids = [] - for _ in dvc.brancher(revs=revs): - index = Index(dvc) - assert index.stages - ids.append(index.identifier) - - # we get "workspace" as well from the brancher by default - assert len(revs) + 1 == len(ids) - possible_ids = { - True: "dd9cf3a3dc2caac5835e64f258ac2d7b", - False: "b446f3cdc35539884dcc474d90ac6286", - } - assert set(ids) == {possible_ids[os.name == "posix"]} - - def test_skip_graph_checks(dvc, mocker): # See https://github.com/iterative/dvc/issues/2671 for more info mock_build_graph = mocker.spy(Index.graph, "fget") @@ -293,18 +191,6 @@ def test_used_objs(tmp_dir, scm, dvc, run_copy, rev): } -def test_getitem(tmp_dir, dvc, run_copy): - (stage1,) = tmp_dir.dvc_gen("foo", "foo") - stage2 = run_copy("foo", "bar", name="copy-foo-bar") - - index = Index(dvc) - assert index["foo.dvc"] == stage1 - assert index["copy-foo-bar"] == stage2 - - with pytest.raises(KeyError): - _ = index["no-valid-stage-name"] - - def test_view_granular_dir(tmp_dir, scm, dvc, run_copy): tmp_dir.dvc_gen( {"dir": {"subdir": {"in-subdir": "in-subdir"}, "in-dir": "in-dir"}},