Stone_Red/dvc
1
0
Fork 0
mirror of https://github.com/Stone-Red-Code/dvc.git synced 2026-11-02 07:18:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

run: Prevent argument duplication with deps/outs

Browse Source 
Fix #1255
Fix #1168
Close #1259

RFC:
- Add / fix docstrings and privatize methods
- Add `__repr__` method for core objects
- Change the name of methods to make them more readable
This commit is contained in:
Mr. Outis
2018-12-11 14:54:45 -06:00
parent a67988a80e
commit 9971f7a624
9 changed files with 137 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dvc/dependency/base.py
+6
View File
@@ -31,6 +31,12 @@ class DependencyBase(object):
self.project = stage.project
self.path = path
def __repr__(self):
return "{class_name}: '{path}'".format(
class_name=type(self).__name__,
path=(self.path or 'No path')
)
def __str__(self):
return self.path
dvc/exceptions.py
+7
View File
@@ -50,6 +50,13 @@ class CircularDependencyError(DvcException):
super(CircularDependencyError, self).__init__(msg.format(dependency))
class ArgumentDuplicationError(DvcException):
def __init__(self, path):
assert isinstance(path, str)
msg = "File '{}' is specified more than once."
super(ArgumentDuplicationError, self).__init__(msg.format(path))
class MoveNotDataSourceError(DvcException):
def __init__(self, path):
msg = "Move is not permitted for stages that are not data sources. " \
dvc/project.py
+21 -31
View File
@@ -58,6 +58,9 @@ class Project(object):
self.updater.check()
def __repr__(self):
return "Project: '{root_dir}'".format(root_dir=self.root_dir)
@staticmethod
def _find_root(root=None):
if root is None:
@@ -209,17 +212,6 @@ class Project(object):
stages = [o.stage.relpath, out.stage.relpath]
raise OutputDuplicationError(o.path, stages)
def _check_circular_dependency(self, deps, outs):
from dvc.exceptions import CircularDependencyError
circular_dependencies = (
set(file.path for file in deps) &
set(file.path for file in outs)
)
if circular_dependencies:
raise CircularDependencyError(circular_dependencies.pop())
def add(self, fname, recursive=False):
from dvc.stage import Stage
@@ -244,9 +236,9 @@ class Project(object):
self._files_to_git_add = []
with self.state:
for f in fnames:
stage = Stage.loads(project=self,
outs=[f],
add=True)
stage = Stage.create(project=self,
outs=[f],
add=True)
if stage is None:
stages.append(stage)
@@ -378,17 +370,17 @@ class Project(object):
from dvc.stage import Stage
with self.state:
stage = Stage.loads(project=self,
fname=fname,
cmd=cmd,
cwd=cwd,
outs=outs,
outs_no_cache=outs_no_cache,
metrics_no_cache=metrics_no_cache,
deps=deps,
overwrite=overwrite,
ignore_build_cache=ignore_build_cache,
remove_outs=remove_outs)
stage = Stage.create(project=self,
fname=fname,
cmd=cmd,
cwd=cwd,
outs=outs,
outs_no_cache=outs_no_cache,
metrics_no_cache=metrics_no_cache,
deps=deps,
overwrite=overwrite,
ignore_build_cache=ignore_build_cache,
remove_outs=remove_outs)
if stage is None:
return None
@@ -397,7 +389,6 @@ class Project(object):
self._check_cwd_specified_as_output(cwd, all_stages)
self._check_output_duplication(stage.outs, all_stages)
self._check_circular_dependency(stage.deps, stage.outs)
self._files_to_git_add = []
with self.state:
@@ -413,16 +404,15 @@ class Project(object):
def imp(self, url, out):
from dvc.stage import Stage
stage = Stage.loads(project=self,
cmd=None,
deps=[url],
outs=[out])
stage = Stage.create(project=self,
cmd=None,
deps=[url],
outs=[out])
if stage is None:
return None
self._check_output_duplication(stage.outs, self.stages())
self._check_circular_dependency(stage.deps, stage.outs)
self._files_to_git_add = []
with self.state:
dvc/remote/base.py
+6
View File
@@ -42,6 +42,12 @@ class RemoteBase(object):
def __init__(self, project, config):
pass
def __repr__(self):
return "{class_name}: '{url}'".format(
class_name=type(self).__name__,
url=(self.url or 'No url')
)
def compat_config(config):
return config.copy()
dvc/remote/http.py
+1
View File
@@ -33,6 +33,7 @@ class RemoteHTTP(RemoteBase):
def __init__(self, project, config):
self.project = project
self.cache_dir = config.get(Config.SECTION_REMOTE_URL)
self.url = self.cache_dir
@property
def prefix(self):
dvc/scm.py
+6
View File
@@ -18,6 +18,12 @@ class Base(object):
self.project = project
self.root_dir = root_dir
def __repr__(self):
return "{class_name}: '{directory}'".format(
class_name=type(self).__name__,
directory=self.dir
)
@property
def dir(self):
return None
dvc/stage.py
+63 -21
View File
@@ -113,12 +113,20 @@ class Stage(object):
self.md5 = md5
self.locked = locked
def __repr__(self):
return "Stage: '{path}'".format(
path=self.relpath if self.path else 'No path'
)
@property
def relpath(self):
return os.path.relpath(self.path)
@property
def is_data_source(self):
"""
Whether the stage file was created with `dvc add`.
"""
return self.cmd is None
@staticmethod
@@ -147,13 +155,20 @@ class Stage(object):
@property
def is_callback(self):
"""
A callback stage is always considered as changed,
so it runs on every `dvc repro` call.
"""
return not self.is_data_source and len(self.deps) == 0
@property
def is_import(self):
return not self.cmd and \
len(self.deps) == 1 and \
len(self.outs) == 1
"""
Whether the stage file was created with `dvc import`.
"""
return (not self.cmd
and len(self.deps) == 1
and len(self.outs) == 1)
def _changed_deps(self, log):
if self.locked:
@@ -210,6 +225,9 @@ class Stage(object):
return ret
def remove_outs(self, ignore_remove=False):
"""
Used mainly for `dvc remove --outs`
"""
for out in self.outs:
out.remove(ignore_remove=ignore_remove)
@@ -254,7 +272,7 @@ class Stage(object):
raise StageFileFormatError(fname, exc)
@staticmethod
def loadd(project, d, path):
def _loadd(project, d, path):
Stage.validate(d, fname=os.path.relpath(path))
path = os.path.abspath(path)
cwd = os.path.dirname(path)
@@ -303,10 +321,10 @@ class Stage(object):
if not os.path.realpath(cwd).startswith(proj_dir):
raise StageBadCwdError(cwd)
@property
def is_cached(self):
"""
Checks if this stage has been already ran and saved to the same
dvc file.
Checks if this stage has been already ran and stored
"""
from dvc.remote.local import RemoteLOCAL
from dvc.remote.s3 import RemoteS3
@@ -335,19 +353,19 @@ class Stage(object):
return old_d == new_d
@staticmethod
def loads(project=None,
cmd=None,
deps=[],
outs=[],
outs_no_cache=[],
metrics_no_cache=[],
fname=None,
cwd=os.curdir,
locked=False,
add=False,
overwrite=True,
ignore_build_cache=False,
remove_outs=False):
def create(project=None,
cmd=None,
deps=[],
outs=[],
outs_no_cache=[],
metrics_no_cache=[],
fname=None,
cwd=os.curdir,
locked=False,
add=False,
overwrite=True,
ignore_build_cache=False,
remove_outs=False):
stage = Stage(project=project,
cwd=cwd,
@@ -360,6 +378,9 @@ class Stage(object):
use_cache=False, metric=True)
stage.deps = dependency.loads_from(stage, deps)
stage._check_circular_dependency()
stage._check_duplicated_arguments()
if fname is not None and os.path.basename(fname) != fname:
msg = "Stage file name '{}' should not contain subdirectories. " \
"Use '-c|--cwd' to change location of the stage file."
@@ -385,7 +406,7 @@ class Stage(object):
stage.unprotect_outs()
if os.path.exists(path):
if not ignore_build_cache and stage.is_cached():
if not ignore_build_cache and stage.is_cached:
Logger.info('Stage is cached, skipping.')
return None
@@ -423,7 +444,7 @@ class Stage(object):
raise StageFileIsNotDvcFileError(fname)
with open(fname, 'r') as fd:
return Stage.loadd(project, yaml.safe_load(fd) or dict(), fname)
return Stage._loadd(project, yaml.safe_load(fd) or dict(), fname)
def dumpd(self):
ret = {}
@@ -505,6 +526,27 @@ class Stage(object):
"https://github.com/iterative/dvc/issues/1307. "
self.project.logger.warn(msg)
def _check_circular_dependency(self):
from dvc.exceptions import CircularDependencyError
circular_dependencies = (
set(file.path for file in self.deps) &
set(file.path for file in self.outs)
)
if circular_dependencies:
raise CircularDependencyError(circular_dependencies.pop())
def _check_duplicated_arguments(self):
from dvc.exceptions import ArgumentDuplicationError
from collections import Counter
count = Counter(file.path for file in self.deps + self.outs)
for file, occurrence in count.items():
if occurrence > 1:
raise ArgumentDuplicationError(file)
def _run(self):
self._check_missing_deps()
executable = os.getenv('SHELL') if os.name != 'nt' else None
dvc/state.py
+3 -8
View File
@@ -74,10 +74,6 @@ class State(object):
def __exit__(self, type, value, tb):
self.dump()
@staticmethod
def init(project):
return State(project)
def _collect(self, path):
if os.path.isdir(path):
return self.project.cache.local.collect_dir_cache(path)
@@ -124,7 +120,7 @@ class State(object):
assert n >= 0
return n
def _load(self, empty=False):
def _prepare_db(self, empty=False):
from dvc import VERSION
if not empty:
@@ -183,7 +179,7 @@ class State(object):
# Try loading once to check that the file is indeed a database
# and reformat it if it is not.
try:
self._load(empty=empty)
self._prepare_db(empty=empty)
return
except sqlite3.DatabaseError:
self.c.close()
@@ -268,8 +264,7 @@ class State(object):
def _do_update(self, path):
"""
Search for the current inode on the table or update it with
gathered information.
Make sure the stored info for the given path is up to date.
"""
if not os.path.exists(path):
return (None, None)
tests/test_run.py
+24
View File
@@ -11,6 +11,7 @@ from dvc.stage import StageFileBadNameError, MissingDep
from dvc.stage import StageBadCwdError, StageFileAlreadyExistsError
from dvc.exceptions import (OutputDuplicationError,
CircularDependencyError,
ArgumentDuplicationError,
WorkingDirectoryAsOutputError)
from tests.basic_env import TestDvc
@@ -121,6 +122,29 @@ class TestRunCircularDependency(TestDvc):
fname='circular-dependency.dvc')
class TestRunDuplicatedArguments(TestDvc):
def test(self):
with self.assertRaises(ArgumentDuplicationError):
self.dvc.run(cmd='',
deps=[],
outs=[self.FOO, self.FOO],
fname='circular-dependency.dvc')
def test_outs_no_cache(self):
with self.assertRaises(ArgumentDuplicationError):
self.dvc.run(cmd='',
outs=[self.FOO],
outs_no_cache=[self.FOO],
fname='circular-dependency.dvc')
def test_non_normalized_paths(self):
with self.assertRaises(ArgumentDuplicationError):
self.dvc.run(cmd='',
deps=[],
outs=['foo', './foo'],
fname='circular-dependency.dvc')
class TestRunWorkingDirectoryAsOutput(TestDvc):
def test(self):
self.dvc.run(cmd='',