Skip to content

Commit 7be5483

Browse files
yarikopticclaude
andcommitted
Enhance path validation error messages
Improve error messages in move.py to provide clearer guidance: - Explain why paths cannot navigate above Dandiset root - Add hints for file vs directory path requirements - Clarify "Cannot move current working directory" restriction - Reference 'dandi ls' command for checking remote paths - Explain dandiset.yaml requirement with actionable hints Update test assertions to match improved error messages. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
1 parent a3dd9f9 commit 7be5483

File tree

2 files changed

+41
-10
lines changed

2 files changed

+41
-10
lines changed

dandi/move.py

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from typing import NewType
1414

1515
from . import get_logger
16-
from .consts import DandiInstance
16+
from .consts import DandiInstance, dandiset_metadata_file
1717
from .dandiapi import DandiAPIClient, RemoteAsset, RemoteDandiset
1818
from .dandiarchive import DandisetURL, parse_dandi_url
1919
from .dandiset import Dandiset
@@ -233,7 +233,11 @@ def resolve(self, path: str) -> tuple[AssetPath, bool]:
233233
posixpath.normpath(posixpath.join(self.subpath.as_posix(), path))
234234
)
235235
if p.parts and p.parts[0] == os.pardir:
236-
raise ValueError(f"{path!r} is outside of Dandiset")
236+
raise ValueError(
237+
f"{path!r} is outside of Dandiset. "
238+
"Paths cannot use '..' to navigate above the Dandiset root. "
239+
"All assets must remain within the Dandiset directory structure."
240+
)
237241
return (AssetPath(str(p)), path.endswith("/"))
238242

239243
def calculate_moves(
@@ -472,11 +476,17 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder:
472476
rpath, needs_dir = self.resolve(path)
473477
p = self.dandiset_path / rpath
474478
if not os.path.lexists(p):
475-
raise NotFoundError(f"No asset at local path {path!r}")
479+
raise NotFoundError(
480+
f"No asset at local path {path!r}. "
481+
"Verify the path is correct and the file exists locally."
482+
)
476483
if p.is_dir():
477484
if is_src:
478485
if p == self.dandiset_path / self.subpath:
479-
raise ValueError("Cannot move current working directory")
486+
raise ValueError(
487+
"Cannot move current working directory. "
488+
"Change to a different directory before moving this location."
489+
)
480490
files = [
481491
df.filepath.relative_to(p).as_posix()
482492
for df in find_dandi_files(
@@ -488,7 +498,10 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder:
488498
files = []
489499
return Folder(rpath, files)
490500
elif needs_dir:
491-
raise ValueError(f"Local path {path!r} is a file")
501+
raise ValueError(
502+
f"Local path {path!r} is a file but a directory was expected. "
503+
"Use a path ending with '/' for directories."
504+
)
492505
else:
493506
return File(rpath)
494507

@@ -612,7 +625,10 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder:
612625
file_found = False
613626
if rpath == self.subpath.as_posix():
614627
if is_src:
615-
raise ValueError("Cannot move current working directory")
628+
raise ValueError(
629+
"Cannot move current working directory. "
630+
"Change to a different directory before moving this location."
631+
)
616632
else:
617633
return Folder(rpath, [])
618634
for p in self.assets.keys():
@@ -629,7 +645,10 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder:
629645
if relcontents:
630646
return Folder(rpath, relcontents)
631647
if needs_dir and file_found:
632-
raise ValueError(f"Remote path {path!r} is a file")
648+
raise ValueError(
649+
f"Remote path {path!r} is a file but a directory was expected. "
650+
"Use a path ending with '/' for directories."
651+
)
633652
elif (
634653
not needs_dir
635654
and not is_src
@@ -641,7 +660,11 @@ def get_path(self, path: str, is_src: bool = True) -> File | Folder:
641660
# remote directory.
642661
return Folder(rpath, [])
643662
else:
644-
raise NotFoundError(f"No asset at remote path {path!r}")
663+
raise NotFoundError(
664+
f"No asset at remote path {path!r}. "
665+
"Verify the path is correct and the asset exists on the server. "
666+
"Use 'dandi ls' to list available assets."
667+
)
645668

646669
def is_dir(self, path: AssetPath) -> bool:
647670
"""Returns true if the given path points to a directory"""
@@ -891,7 +914,11 @@ def find_dandiset_and_subpath(path: Path) -> tuple[Dandiset, Path]:
891914
path = path.absolute()
892915
ds = Dandiset.find(path)
893916
if ds is None:
894-
raise ValueError(f"{path}: not a Dandiset")
917+
raise ValueError(
918+
f"{path}: not a Dandiset. "
919+
f"The directory does not contain a '{dandiset_metadata_file}' file. "
920+
"Use 'dandi download' to download a dandiset first."
921+
)
895922
return (ds, path.relative_to(ds.path))
896923

897924

dandi/tests/test_move.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,11 @@ def test_move_not_dandiset(
769769
monkeypatch.chdir(tmp_path)
770770
with pytest.raises(ValueError) as excinfo:
771771
move("file.txt", "subdir2/banana.txt", dest="subdir1", work_on=work_on)
772-
assert str(excinfo.value) == f"{tmp_path.absolute()}: not a Dandiset"
772+
assert str(excinfo.value) == (
773+
f"{tmp_path.absolute()}: not a Dandiset. "
774+
"The directory does not contain a 'dandiset.yaml' file. "
775+
"Use 'dandi download' to download a dandiset first."
776+
)
773777

774778

775779
def test_move_local_delete_empty_dirs(

0 commit comments

Comments
 (0)