diff --git a/dandi/move.py b/dandi/move.py index 311a41c7a..72806dcd4 100644 --- a/dandi/move.py +++ b/dandi/move.py @@ -24,7 +24,7 @@ from typing import NewType from . import get_logger -from .consts import DandiInstance +from .consts import DandiInstance, dandiset_metadata_file from .dandiapi import DandiAPIClient, RemoteAsset, RemoteDandiset from .dandiarchive import DandisetURL, parse_dandi_url from .dandiset import Dandiset @@ -244,7 +244,11 @@ def resolve(self, path: str) -> tuple[AssetPath, bool]: posixpath.normpath(posixpath.join(self.subpath.as_posix(), path)) ) if p.parts and p.parts[0] == os.pardir: - raise ValueError(f"{path!r} is outside of Dandiset") + raise ValueError( + f"{path!r} is outside of Dandiset. " + "Paths cannot use '..' to navigate above the Dandiset root. " + "All assets must remain within the Dandiset directory structure." + ) return (AssetPath(str(p)), path.endswith("/")) def calculate_moves( @@ -483,11 +487,17 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder: rpath, needs_dir = self.resolve(path) p = self.dandiset_path / rpath if not os.path.lexists(p): - raise NotFoundError(f"No asset at local path {path!r}") + raise NotFoundError( + f"No asset at local path {path!r}. " + "Verify the path is correct and the file exists locally." + ) if p.is_dir(): if is_src: if p == self.dandiset_path / self.subpath: - raise ValueError("Cannot move current working directory") + raise ValueError( + "Cannot move current working directory. " + "Change to a different directory before moving this location." + ) files = [ df.filepath.relative_to(p).as_posix() for df in find_dandi_files( @@ -499,7 +509,10 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder: files = [] return Folder(rpath, files) elif needs_dir: - raise ValueError(f"Local path {path!r} is a file") + raise ValueError( + f"Local path {path!r} is a file but a directory was expected. " + "Use a path ending with '/' for directories." + ) else: return File(rpath) @@ -623,7 +636,10 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder: file_found = False if rpath == self.subpath.as_posix(): if is_src: - raise ValueError("Cannot move current working directory") + raise ValueError( + "Cannot move current working directory. " + "Change to a different directory before moving this location." + ) else: return Folder(rpath, []) for p in self.assets.keys(): @@ -640,7 +656,10 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder: if relcontents: return Folder(rpath, relcontents) if needs_dir and file_found: - raise ValueError(f"Remote path {path!r} is a file") + raise ValueError( + f"Remote path {path!r} is a file but a directory was expected. " + "Use a path ending with '/' for directories." + ) elif ( not needs_dir and not is_src @@ -652,7 +671,11 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder: # remote directory. return Folder(rpath, []) else: - raise NotFoundError(f"No asset at remote path {path!r}") + raise NotFoundError( + f"No asset at remote path {path!r}. " + "Verify the path is correct and the asset exists on the server. " + "Use 'dandi ls' to list available assets." + ) def is_dir(self, path: AssetPath) -> bool: """Returns true if the given path points to a directory""" @@ -902,7 +925,11 @@ def find_dandiset_and_subpath(path: Path) -> tuple[Dandiset, Path]: path = path.absolute() ds = Dandiset.find(path) if ds is None: - raise ValueError(f"{path}: not a Dandiset") + raise ValueError( + f"{path}: not a Dandiset. " + f"The directory does not contain a '{dandiset_metadata_file}' file. " + "Use 'dandi download' to download a dandiset first." + ) return (ds, path.relative_to(ds.path)) diff --git a/dandi/tests/test_move.py b/dandi/tests/test_move.py index 625bef307..7b8c99ede 100644 --- a/dandi/tests/test_move.py +++ b/dandi/tests/test_move.py @@ -374,9 +374,15 @@ def test_move_nonexistent_src( work_on=work_on, dandi_instance=moving_dandiset.api.instance_id, ) - assert ( - str(excinfo.value) - == f"No asset at {'remote' if work_on == 'remote' else 'local'} path 'subdir1/avocado.txt'" + asset_type = "remote" if work_on == MoveWorkOn.REMOTE else "local" + assert str(excinfo.value) == ( + f"No asset at {asset_type} path 'subdir1/avocado.txt'. " + + ( + "Verify the path is correct and the asset exists on the server. " + "Use 'dandi ls' to list available assets." + if work_on == MoveWorkOn.REMOTE + else "Verify the path is correct and the file exists locally." + ) ) check_assets(moving_dandiset, starting_assets, work_on, {}) @@ -400,9 +406,10 @@ def test_move_file_slash_src( work_on=work_on, dandi_instance=moving_dandiset.api.instance_id, ) - assert ( - str(excinfo.value) - == f"{'Remote' if work_on == 'remote' else 'Local'} path 'subdir1/apple.txt/' is a file" + path_type = "Remote" if work_on == MoveWorkOn.REMOTE else "Local" + assert str(excinfo.value) == ( + f"{path_type} path 'subdir1/apple.txt/' is a file but a directory " + "was expected. Use a path ending with '/' for directories." ) check_assets(moving_dandiset, starting_assets, work_on, {}) @@ -425,9 +432,10 @@ def test_move_file_slash_dest( work_on=work_on, dandi_instance=moving_dandiset.api.instance_id, ) - assert ( - str(excinfo.value) - == f"{'Remote' if work_on == 'remote' else 'Local'} path 'subdir1/apple.txt/' is a file" + path_type = "Remote" if work_on == MoveWorkOn.REMOTE else "Local" + assert str(excinfo.value) == ( + f"{path_type} path 'subdir1/apple.txt/' is a file but a directory " + "was expected. Use a path ending with '/' for directories." ) check_assets(moving_dandiset, starting_assets, work_on, {}) @@ -593,9 +601,14 @@ def test_move_from_subdir_abspaths( work_on=work_on, dandi_instance=moving_dandiset.api.instance_id, ) - assert ( - str(excinfo.value) - == f"No asset at {'remote' if work_on == 'remote' else 'local'} path 'file.txt'" + assert str(excinfo.value) == ( + f"No asset at {'remote' if work_on == MoveWorkOn.REMOTE else 'local'} path 'file.txt'. " + + ( + "Verify the path is correct and the asset exists on the server. " + "Use 'dandi ls' to list available assets." + if work_on == MoveWorkOn.REMOTE + else "Verify the path is correct and the file exists locally." + ) ) check_assets(moving_dandiset, starting_assets, work_on, {}) @@ -619,7 +632,11 @@ def test_move_from_subdir_as_dot( dandi_instance=moving_dandiset.api.instance_id, devel_debug=True, ) - assert str(excinfo.value) == "Cannot move current working directory" + assert ( + str(excinfo.value) + == "Cannot move current working directory. " + "Change to a different directory before moving this location." + ) check_assets(moving_dandiset, starting_assets, work_on, {}) @@ -769,7 +786,11 @@ def test_move_not_dandiset( monkeypatch.chdir(tmp_path) with pytest.raises(ValueError) as excinfo: move("file.txt", "subdir2/banana.txt", dest="subdir1", work_on=work_on) - assert str(excinfo.value) == f"{tmp_path.absolute()}: not a Dandiset" + assert str(excinfo.value) == ( + f"{tmp_path.absolute()}: not a Dandiset. " + "The directory does not contain a 'dandiset.yaml' file. " + "Use 'dandi download' to download a dandiset first." + ) def test_move_local_delete_empty_dirs(