From 8fd18f5017b4ffa78c74de51d123cc4099584402 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Thu, 4 Jul 2019 23:35:58 -0700 Subject: [PATCH] url info and repo grap optimization: address review comments --- dvc/path_info.py | 2 +- dvc/repo/__init__.py | 17 ++++++---------- tests/unit/test_path_info.py | 38 +++++++++++++++++++++--------------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/dvc/path_info.py b/dvc/path_info.py index c28477701..bd18b52bf 100644 --- a/dvc/path_info.py +++ b/dvc/path_info.py @@ -213,7 +213,7 @@ class URLInfo(object): @property def parts(self): - return (self.scheme, self.parsed.netloc) + self._path.parts + return (self.scheme, self.netloc) + self._path.parts @property def bucket(self): diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 7f3fc2888..f4ac80595 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -3,6 +3,8 @@ from __future__ import unicode_literals import os import logging +from itertools import chain + from dvc.config import Config from dvc.exceptions import ( NotDvcRepoError, @@ -305,10 +307,7 @@ class Repo(object): for stage in stages: for out in stage.outs: if out.path_info in outs: - stages = [ - stage.relpath, - outs.get(out.path_info).stage.relpath, - ] + stages = [stage.relpath, outs[out.path_info].stage.relpath] raise OutputDuplicationError(str(out), stages) outs[out.path_info] = out @@ -316,15 +315,11 @@ class Repo(object): for out in stage.outs: for p in out.path_info.parents: if p in outs: - raise OverlappingOutputPathsError( - outs.get(p), outs.get(out.path_info) - ) + raise OverlappingOutputPathsError(outs[p], out) for stage in stages: stage_path_info = PathInfo(stage.path) - if stage_path_info in outs: - raise StagePathAsOutputError(stage.wdir, stage.relpath) - for p in stage_path_info.parents: + for p in chain([stage_path_info], stage_path_info.parents): if p in outs: raise StagePathAsOutputError(stage.wdir, stage.relpath) @@ -341,7 +336,7 @@ class Repo(object): or dep.path_info.isin(out) or out.isin(dep.path_info) ): - dep_stage = outs.get(out).stage + dep_stage = outs[out].stage dep_node = relpath(dep_stage.path, self.root_dir) G.add_node(dep_node, stage=dep_stage) G.add_edge(node, dep_node) diff --git a/tests/unit/test_path_info.py b/tests/unit/test_path_info.py index 72479f9f1..46bd032b9 100644 --- a/tests/unit/test_path_info.py +++ b/tests/unit/test_path_info.py @@ -1,30 +1,36 @@ -from dvc.path_info import URLInfo +import pytest + +from dvc.path_info import URLInfo, CloudURLInfo -def test_url_info_str(): - u = URLInfo("ssh://user@test.com:/test1/") +@pytest.mark.parametrize("cls", [URLInfo, CloudURLInfo]) +def test_url_info_str(cls): + u = cls("ssh://user@test.com:/test1/") assert u.url == "ssh://user@test.com/test1/" assert str(u) == u.url -def test_url_info_eq(): - u1 = URLInfo("ssh://user@test.com:/test1/") - u2 = URLInfo("ssh://user@test.com/test1") +@pytest.mark.parametrize("cls", [URLInfo, CloudURLInfo]) +def test_url_info_eq(cls): + u1 = cls("ssh://user@test.com:/test1/") + u2 = cls("ssh://user@test.com/test1") assert u1 == u2 -def test_url_info_parent(): - u1 = URLInfo("ssh://user@test.com:/test1/test2") +@pytest.mark.parametrize("cls", [URLInfo, CloudURLInfo]) +def test_url_info_parent(cls): + u1 = cls("ssh://user@test.com:/test1/test2") p = u1.parent - u3 = URLInfo("ssh://user@test.com/test1") + u3 = cls("ssh://user@test.com/test1") assert u3 == p assert str(p) == "ssh://user@test.com/test1" -def test_url_info_parents(): - u1 = URLInfo("ssh://user@test.com:/test1/test2/test3") - parents = u1.parents - assert len(parents) == 3 - assert parents[0] == URLInfo("ssh://user@test.com/test1/test2") - assert parents[1] == URLInfo("ssh://user@test.com/test1") - assert parents[2] == URLInfo("ssh://user@test.com/") +@pytest.mark.parametrize("cls", [URLInfo, CloudURLInfo]) +def test_url_info_parents(cls): + u1 = cls("ssh://user@test.com:/test1/test2/test3") + assert list(u1.parents) == [ + cls("ssh://user@test.com/test1/test2"), + cls("ssh://user@test.com/test1"), + cls("ssh://user@test.com/"), + ]