From c120c8b663efb4dd399257a319eda407e2898de4 Mon Sep 17 00:00:00 2001 From: pared Date: Tue, 15 Jan 2019 19:05:16 +0100 Subject: [PATCH] move: handle directory as target argument --- dvc/project.py | 25 ++++++++++---- tests/test_move.py | 74 ++++++++++++++++++++++++++++++++++++++++- tests/utils/__init__.py | 6 ++++ 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/dvc/project.py b/dvc/project.py index 68487ff62..ed1affbe4 100644 --- a/dvc/project.py +++ b/dvc/project.py @@ -282,6 +282,8 @@ class Project(object): from_out = Output.loads_from(Stage(self, cwd=os.curdir), [from_path])[0] + to_path = self._expand_target_path(from_path, to_path) + try: stage, out = next((stage, out) for stage in self.stages() @@ -294,22 +296,26 @@ class Project(object): if not stage.is_data_source: raise MoveNotDataSourceError(stage.relpath) - to_out = Output.loads_from(stage, [to_path], out.cache, out.metric)[0] - - with self.state: - out.move(to_out) - stage_name = os.path.splitext(os.path.basename(stage.path))[0] from_name = os.path.basename(from_out.path) - if stage_name == from_name: os.unlink(stage.path) stage.path = os.path.join( - os.path.dirname(to_out.path), + os.path.dirname(to_path), os.path.basename(to_path) + Stage.STAGE_FILE_SUFFIX ) + stage.cwd = os.path.join(self.root_dir, os.path.dirname(to_path)) + + to_out = Output.loads_from(stage, + [os.path.basename(to_path)], + out.cache, + out.metric)[0] + + with self.state: + out.move(to_out) + stage.dump() self._remind_to_git_add() @@ -1293,3 +1299,8 @@ class Project(object): blue=colorama.Fore.BLUE, nc=colorama.Fore.RESET) ) + + def _expand_target_path(self, from_path, to_path): + if os.path.isdir(to_path) and not os.path.isdir(from_path): + return os.path.join(to_path, os.path.basename(from_path)) + return to_path diff --git a/tests/test_move.py b/tests/test_move.py index 088eaa07c..8b651cd2d 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -6,6 +6,7 @@ from dvc.stage import Stage from tests.basic_env import TestDvc from tests.test_repro import TestRepro +from tests.utils import load_stage_file class TestMove(TestDvc): @@ -86,7 +87,7 @@ class TestMoveFileToDirectory(TestDvc): self.assertEqual(ret, 0) self.assertTrue(os.path.exists(foo_dvc_file)) - new_foo_path = self.DATA_DIR+"/"+self.FOO + new_foo_path = os.path.join(self.DATA_DIR, self.FOO) new_foo_dvc_path = new_foo_path + Stage.STAGE_FILE_SUFFIX ret = main(["move", self.FOO, new_foo_path]) self.assertEqual(ret, 0) @@ -95,3 +96,74 @@ class TestMoveFileToDirectory(TestDvc): self.assertFalse(os.path.exists(foo_dvc_file)) self.assertTrue(os.path.exists(new_foo_path)) self.assertTrue(os.path.exists(new_foo_dvc_path)) + + +class TestMoveFileToDirectoryWithoutSpecifiedTargetName(TestDvc): + def test(self): + foo_stage_file_path = self.FOO + Stage.STAGE_FILE_SUFFIX + ret = main(['add', self.FOO]) + self.assertEqual(ret, 0) + self.assertTrue(os.path.exists(foo_stage_file_path)) + + target_foo_path = os.path.join(self.DATA_DIR, self.FOO) + target_foo_stage_file_path = target_foo_path + Stage.STAGE_FILE_SUFFIX + + ret = main(['move', self.FOO, self.DATA_DIR]) + self.assertEqual(ret, 0) + + self.assertFalse(os.path.exists(self.FOO)) + self.assertFalse(os.path.exists(foo_stage_file_path)) + + self.assertTrue(os.path.exists(target_foo_path)) + self.assertTrue(os.path.exists(target_foo_stage_file_path)) + + new_stage = load_stage_file(target_foo_stage_file_path) + self.assertEqual(self.FOO, new_stage['outs'][0]['path']) + + +class TestMoveDirectoryShouldNotOverwriteExisting(TestDvc): + def test(self): + new_dir_name = 'data_dir2' + os.mkdir(new_dir_name) + ret = main(['add', self.DATA_DIR]) + self.assertEqual(ret, 0) + + ret = main(['move', self.DATA_DIR, new_dir_name]) + self.assertEqual(ret, 0) + + self.assertFalse(os.path.exists(self.DATA_DIR)) + self.assertFalse( + os.path.exists(self.DATA_DIR + Stage.STAGE_FILE_SUFFIX)) + + self.assertTrue(os.path.exists(new_dir_name)) + self.assertTrue(os.path.isfile(new_dir_name + Stage.STAGE_FILE_SUFFIX)) + + self.assertEqual(os.listdir(new_dir_name), + [self.DATA_DIR]) + + +class TestMoveFileBetweenDirectories(TestDvc): + def test(self): + data_stage_file = self.DATA + Stage.STAGE_FILE_SUFFIX + ret = main(['add', self.DATA]) + self.assertEqual(ret, 0) + self.assertTrue(os.path.exists(data_stage_file)) + + new_data_dir = 'data_dir2' + os.makedirs(new_data_dir) + + ret = main(['move', self.DATA, new_data_dir]) + self.assertEqual(ret, 0) + + new_data_path = os.path.join(new_data_dir, os.path.basename(self.DATA)) + new_data_stage_file = new_data_path + Stage.STAGE_FILE_SUFFIX + + self.assertFalse(os.path.exists(self.DATA)) + self.assertFalse(os.path.exists(data_stage_file)) + + self.assertTrue(os.path.exists(new_data_path)) + self.assertTrue(os.path.exists(new_data_stage_file)) + + new_stage_file = load_stage_file(new_data_stage_file) + self.assertEqual(os.path.basename(self.DATA), + new_stage_file['outs'][0]['path']) diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py index 0867f7924..cbb6bf558 100644 --- a/tests/utils/__init__.py +++ b/tests/utils/__init__.py @@ -1,3 +1,4 @@ +import yaml from mock import MagicMock @@ -10,3 +11,8 @@ def spy(method_to_decorate): wrapper.mock = mock return wrapper + + +def load_stage_file(path): + with open(path, 'r') as fobj: + return yaml.safe_load(fobj)