diff --git a/dvc/repo/collect.py b/dvc/repo/collect.py index cfa503806..a2491f0fa 100644 --- a/dvc/repo/collect.py +++ b/dvc/repo/collect.py @@ -1,6 +1,6 @@ import logging import os -from typing import TYPE_CHECKING, Callable, Iterable, Set, Tuple +from typing import TYPE_CHECKING, Callable, Iterable, List, Tuple from dvc.path_info import PathInfo from dvc.types import DvcPath @@ -13,19 +13,19 @@ logger = logging.getLogger(__name__) FilterFn = Callable[["BaseOutput"], bool] -Outputs = Set["BaseOutput"] -DvcPaths = Set[DvcPath] +Outputs = List["BaseOutput"] +DvcPaths = List[DvcPath] def _collect_outs( repo: "Repo", output_filter: FilterFn = None, deps: bool = False, ) -> Outputs: - outs = { + outs = [ out for stage in repo.graph # using `graph` to ensure graph checks run for out in (stage.deps if deps else stage.outs) - } - return set(filter(output_filter, outs)) if output_filter else outs + ] + return list(filter(output_filter, outs)) if output_filter else outs def _collect_paths( @@ -36,14 +36,14 @@ def _collect_paths( ): from dvc.tree.repo import RepoTree - path_infos = {PathInfo(os.path.abspath(target)) for target in targets} + path_infos = [PathInfo(os.path.abspath(target)) for target in targets] tree = RepoTree(repo) - target_infos = set() + target_infos = [] for path_info in path_infos: if recursive and tree.isdir(path_info): - target_infos.update(set(tree.walk_files(path_info))) + target_infos.extend(tree.walk_files(path_info)) if not tree.exists(path_info): if not recursive: @@ -51,19 +51,19 @@ def _collect_paths( "'%s' was not found at: '%s'.", path_info, rev, ) continue - target_infos.add(path_info) + target_infos.append(path_info) return target_infos def _filter_duplicates( outs: Outputs, path_infos: DvcPaths ) -> Tuple[Outputs, DvcPaths]: - res_outs: Outputs = set() - res_infos = set(path_infos) + res_outs: Outputs = [] + res_infos = path_infos for out in outs: if out.path_info in path_infos: - res_outs.add(out) + res_outs.append(out) res_infos.remove(out.path_info) return res_outs, res_infos @@ -82,7 +82,7 @@ def collect( outs: Outputs = _collect_outs(repo, output_filter=output_filter, deps=deps) if not targets: - path_infos: DvcPaths = set() + path_infos: DvcPaths = [] return outs, path_infos target_infos = _collect_paths(repo, targets, recursive=recursive, rev=rev) diff --git a/dvc/repo/params/show.py b/dvc/repo/params/show.py index a400a8620..8ea73b8f8 100644 --- a/dvc/repo/params/show.py +++ b/dvc/repo/params/show.py @@ -35,11 +35,13 @@ def _collect_configs(repo: "Repo", rev, targets=None) -> List["DvcPath"]: output_filter=_is_params, rev=rev, ) - path_infos.update({p.path_info for p in params}) + path_infos.extend([p.path_info for p in params]) if not targets: - path_infos.add( + default_params = ( PathInfo(repo.root_dir) / ParamsDependency.DEFAULT_PARAMS_FILE ) + if default_params not in path_infos: + path_infos.append(default_params) return list(path_infos) diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index b85817f9e..d0b795d90 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -102,7 +102,7 @@ class Plots: plots = _prepare_plots(data, revs, props) result = {} - for datafile, desc in sorted(plots.items()): + for datafile, desc in plots.items(): try: result[datafile] = _render( datafile, desc["data"], desc["props"], templates diff --git a/tests/conftest.py b/tests/conftest.py index 72797fe58..68b7d9ba4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -138,3 +138,17 @@ def pytest_configure(config): enabled_remotes.discard(remote_name) if enabled: enabled_remotes.add(remote_name) + + +@pytest.fixture +def dummy_stage(tmp_dir, dvc): + def make(path="dvc.yaml", name="dummy_stage", **kwargs): + from dvc.stage import PipelineStage, create_stage + + stage = create_stage( + PipelineStage, dvc, path, name=name, cmd="", **kwargs + ) + stage.dump() + return stage + + return make diff --git a/tests/unit/test_metrics.py b/tests/unit/test_metrics.py new file mode 100644 index 000000000..eb9f9538b --- /dev/null +++ b/tests/unit/test_metrics.py @@ -0,0 +1,28 @@ +import json +import os + + +def test_metrics_order(tmp_dir, dvc, dummy_stage): + tmp_dir.gen( + { + "p.json": json.dumps({"p1": 1}), + "p1.json": json.dumps({"p2": 1}), + "sub": { + "p3.json": json.dumps({"p3": 1}), + "p4.json": json.dumps({"p4": 1}), + }, + } + ) + + dummy_stage(metrics=["p.json", str(tmp_dir / "sub" / "p4.json")]) + dummy_stage( + path=str(tmp_dir / "sub" / "dvc.yaml"), + metrics=["p1.json", str(tmp_dir / "sub" / "p3.json")], + ) + + assert list(dvc.metrics.show()[""]) == [ + "p.json", + os.path.join("sub", "p4.json"), + "p1.json", + os.path.join("sub", "p3.json"), + ] diff --git a/tests/unit/test_params.py b/tests/unit/test_params.py new file mode 100644 index 000000000..c7ba4fb59 --- /dev/null +++ b/tests/unit/test_params.py @@ -0,0 +1,22 @@ +import os + +from dvc.utils.serialize import dumps_yaml + + +def test_params_order(tmp_dir, dvc, dummy_stage): + tmp_dir.gen( + { + "params.yaml": dumps_yaml({"p1": 1}), + "p1.yaml": dumps_yaml({"p2": 1}), + "sub": {"p2.yaml": dumps_yaml({"p3": 1})}, + } + ) + + p2_path = os.path.join("sub", "p2.yaml") + sub_stage = os.path.join("sub", "dvc.yaml") + + dummy_stage(params=[{p2_path: ["p3"]}, {"p1.yaml": ["p2"]}]) + dummy_stage(path=sub_stage, params=["p1"]) + + # params are sorted during dumping, therefore p1 is first + assert list(dvc.params.show()[""]) == ["p1.yaml", p2_path, "params.yaml"] diff --git a/tests/unit/test_plots.py b/tests/unit/test_plots.py new file mode 100644 index 000000000..c3387a5d0 --- /dev/null +++ b/tests/unit/test_plots.py @@ -0,0 +1,28 @@ +import json +import os + + +def test_plots_order(tmp_dir, dvc, dummy_stage): + tmp_dir.gen( + { + "p.json": json.dumps([{"p1": 1}, {"p1": 2}]), + "p1.json": json.dumps([{"p2": 1}, {"p2": 2}]), + "sub": { + "p3.json": json.dumps([{"p3": 1}, {"p3": 2}]), + "p4.json": json.dumps([{"p4": 1}, {"p4": 2}]), + }, + } + ) + + dummy_stage(plots=["p.json", str(tmp_dir / "sub" / "p4.json")]) + dummy_stage( + path=str(tmp_dir / "sub" / "dvc.yaml"), + plots=["p1.json", str(tmp_dir / "sub" / "p3.json")], + ) + + assert list(dvc.plots.show()) == [ + "p.json", + os.path.join("sub", "p4.json"), + "p1.json", + os.path.join("sub", "p3.json"), + ]