From 112777abc3454e155ef2d62f53627cfd924ab7d5 Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Tue, 6 Aug 2024 09:53:39 -0400 Subject: [PATCH] Fix handling of `--preserve-tree` for asset ID-only URLs --- dandi/cli/cmd_download.py | 3 ++- dandi/download.py | 13 ++++++++----- dandi/tests/test_download.py | 9 +++++++++ docs/source/cmdline/download.rst | 3 ++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/dandi/cli/cmd_download.py b/dandi/cli/cmd_download.py index 32c1ff66d..713c226cc 100644 --- a/dandi/cli/cmd_download.py +++ b/dandi/cli/cmd_download.py @@ -105,7 +105,8 @@ is_flag=True, help=( "When downloading only part of a Dandiset, also download" - " `dandiset.yaml` and do not strip leading directories from asset" + " `dandiset.yaml` (unless downloading an asset URL that does not" + " include a Dandiset ID) and do not strip leading directories from asset" " paths. Implies `--download all`." ), ) diff --git a/dandi/download.py b/dandi/download.py index 2df711c78..7f4001f2f 100644 --- a/dandi/download.py +++ b/dandi/download.py @@ -242,11 +242,14 @@ def download_generator(self) -> Iterator[dict]: with self.url.navigate(strict=True) as (client, dandiset, assets): if ( - isinstance(self.url, DandisetURL) - or self.is_dandiset_yaml() - or self.preserve_tree - ) and self.get_metadata: - assert dandiset is not None + ( + isinstance(self.url, DandisetURL) + or self.is_dandiset_yaml() + or self.preserve_tree + ) + and self.get_metadata + and dandiset is not None + ): for resp in _populate_dandiset_yaml( self.output_path, dandiset, self.existing ): diff --git a/dandi/tests/test_download.py b/dandi/tests/test_download.py index 2a8c044ce..cd74c1609 100644 --- a/dandi/tests/test_download.py +++ b/dandi/tests/test_download.py @@ -250,6 +250,15 @@ def test_download_asset_id_only(text_dandiset: SampleDandiset, tmp_path: Path) - assert (tmp_path / "coconut.txt").read_text() == "Coconut\n" +def test_download_asset_id_only_preserve_tree( + text_dandiset: SampleDandiset, tmp_path: Path +) -> None: + asset = text_dandiset.dandiset.get_asset_by_path("subdir2/coconut.txt") + download(asset.base_download_url, tmp_path, preserve_tree=True) + assert list_paths(tmp_path, dirs=False) == [tmp_path / "subdir2" / "coconut.txt"] + assert (tmp_path / "subdir2" / "coconut.txt").read_text() == "Coconut\n" + + def test_download_asset_by_equal_prefix( text_dandiset: SampleDandiset, tmp_path: Path ) -> None: diff --git a/docs/source/cmdline/download.rst b/docs/source/cmdline/download.rst index b0c6a3760..d942362a4 100644 --- a/docs/source/cmdline/download.rst +++ b/docs/source/cmdline/download.rst @@ -49,7 +49,8 @@ Options .. option:: --preserve-tree When downloading only part of a Dandiset, also download - :file:`dandiset.yaml` and do not strip leading directories from asset + :file:`dandiset.yaml` (unless downloading an asset URL that does not + include a Dandiset ID) and do not strip leading directories from asset paths. Implies ``--download all``. .. option:: --sync