From b10a29906e80f231db2475dba362eeb27850309b Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:20:22 -0700 Subject: [PATCH] Support click >= 8.2.0 click 8.2 made Choice generic and, for an Enum, matches on member names instead of values. The (str, Enum) options in the CLI rely on their lowercase values being the accepted tokens, so add an EnumChoice type that keeps choices as the enum member values and returns the corresponding member, and use it for the affected options. This lets the click < 8.2.0 upper bound be removed. Closes #1631 Co-Authored-By: Claude Opus 4.8 (1M context) --- dandi/cli/base.py | 19 +++++++++ dandi/cli/cmd_download.py | 16 +++++--- dandi/cli/cmd_move.py | 11 ++++-- dandi/cli/cmd_organize.py | 13 +++++-- dandi/cli/cmd_upload.py | 7 ++-- dandi/cli/tests/test_base.py | 74 ++++++++++++++++++++++++++++++++++++ pyproject.toml | 3 +- 7 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 dandi/cli/tests/test_base.py diff --git a/dandi/cli/base.py b/dandi/cli/base.py index 9d8bc6ace..1b118d7ed 100644 --- a/dandi/cli/base.py +++ b/dandi/cli/base.py @@ -1,3 +1,4 @@ +from enum import Enum from functools import wraps import os @@ -29,6 +30,24 @@ def get_metavar(self, param, ctx=None): return "N[:M]" +class EnumChoice(click.Choice): + """A ``click.Choice`` over a ``str``-valued ``Enum``, matched on member values. + + The available choices presented on the command line are the enum member + values (e.g. ``error``, ``skip``), and ``convert`` returns the corresponding + enum member. A string default is converted to its member as well. + """ + + def __init__(self, enum_cls: type[Enum], case_sensitive: bool = True) -> None: + self.enum_cls = enum_cls + super().__init__([e.value for e in enum_cls], case_sensitive=case_sensitive) + + def convert(self, value, param, ctx): + if value is None or isinstance(value, self.enum_cls): + return value + return self.enum_cls(super().convert(value, param, ctx)) + + class ChoiceList(click.ParamType): name = "choice-list" diff --git a/dandi/cli/cmd_download.py b/dandi/cli/cmd_download.py index 4547ae043..7dee42cc4 100644 --- a/dandi/cli/cmd_download.py +++ b/dandi/cli/cmd_download.py @@ -5,7 +5,13 @@ import click -from .base import ChoiceList, IntColonInt, instance_option, map_to_click_exceptions +from .base import ( + ChoiceList, + EnumChoice, + IntColonInt, + instance_option, + map_to_click_exceptions, +) from ..consts import SyncMode from ..dandiarchive import _dandi_url_parser, parse_dandi_url from ..dandiset import Dandiset @@ -63,7 +69,7 @@ @click.option( "-e", "--existing", - type=click.Choice(list(DownloadExisting)), + type=EnumChoice(DownloadExisting), # TODO: verify-reupload (to become default) help="How to handle paths that already exist locally. " "For 'error', if the local file exists, display an error and skip downloading that asset. " @@ -80,12 +86,12 @@ "-f", "--format", help="Choose the format/frontend for output. TODO: support all of the ls", - type=click.Choice(list(DownloadFormat)), + type=EnumChoice(DownloadFormat), default="pyout", ) @click.option( "--path-type", - type=click.Choice(list(PathType)), + type=EnumChoice(PathType), default="exact", help="Whether to interpret asset paths in URLs as exact matches or glob patterns", show_default=True, @@ -121,7 +127,7 @@ is_flag=False, flag_value="ask", default=None, - type=click.Choice(list(SyncMode)), + type=EnumChoice(SyncMode), help="Delete local assets that do not exist on the server. " "With 'ask' (the default when --sync is passed without a value), prompt before " "deleting. With 'do', delete without prompting.", diff --git a/dandi/cli/cmd_move.py b/dandi/cli/cmd_move.py index 6a8898f9a..ebc3e42d2 100644 --- a/dandi/cli/cmd_move.py +++ b/dandi/cli/cmd_move.py @@ -2,7 +2,12 @@ import click -from .base import devel_debug_option, instance_option, map_to_click_exceptions +from .base import ( + EnumChoice, + devel_debug_option, + instance_option, + map_to_click_exceptions, +) from ..move import MoveExisting, MoveWorkOn @@ -16,7 +21,7 @@ @click.option( "-e", "--existing", - type=click.Choice(list(MoveExisting)), + type=EnumChoice(MoveExisting), default="error", help="How to handle assets that would be moved to a destination that already exists", show_default=True, @@ -30,7 +35,7 @@ @click.option( "-w", "--work-on", - type=click.Choice(list(MoveWorkOn)), + type=EnumChoice(MoveWorkOn), default="auto", help=( "Whether to operate on the local Dandiset, remote Dandiset, or both;" diff --git a/dandi/cli/cmd_organize.py b/dandi/cli/cmd_organize.py index f5c1e216c..47930cbab 100644 --- a/dandi/cli/cmd_organize.py +++ b/dandi/cli/cmd_organize.py @@ -2,7 +2,12 @@ import click -from .base import dandiset_path_option, devel_debug_option, map_to_click_exceptions +from .base import ( + EnumChoice, + dandiset_path_option, + devel_debug_option, + map_to_click_exceptions, +) from ..consts import dandi_layout_fields from ..organize import CopyMode, FileOperationMode, OrganizeInvalid @@ -17,7 +22,7 @@ @click.option( "--invalid", help="What to do if files without sufficient metadata are encountered.", - type=click.Choice(list(OrganizeInvalid)), + type=EnumChoice(OrganizeInvalid), default="fail", show_default=True, ) @@ -30,7 +35,7 @@ "If 'auto' - whichever of symlink, hardlink, copy is allowed by system. " "The other modes (copy, move, symlink, hardlink) define how data files " "should be made available.", - type=click.Choice(list(FileOperationMode)), + type=EnumChoice(FileOperationMode), default="auto", show_default=True, ) @@ -45,7 +50,7 @@ ) @click.option( "--media-files-mode", - type=click.Choice(list(CopyMode)), + type=EnumChoice(CopyMode), default=None, help="How to relocate video files referenced by NWB files", ) diff --git a/dandi/cli/cmd_upload.py b/dandi/cli/cmd_upload.py index 25222a38e..9b5f1760c 100644 --- a/dandi/cli/cmd_upload.py +++ b/dandi/cli/cmd_upload.py @@ -3,6 +3,7 @@ import click from .base import ( + EnumChoice, IntColonInt, devel_debug_option, devel_option, @@ -17,7 +18,7 @@ @click.option( "-e", "--existing", - type=click.Choice(list(UploadExisting)), + type=EnumChoice(UploadExisting), help="What to do if a file found existing on the server. 'skip' would skip" "the file, 'force' - force reupload, 'overwrite' - force upload if " "either size or modification time differs; 'refresh' - upload only if " @@ -40,7 +41,7 @@ is_flag=False, flag_value="ask", default=None, - type=click.Choice(list(SyncMode)), + type=EnumChoice(SyncMode), help="Delete assets on the server that do not exist locally. " "With 'ask' (the default when --sync is passed without a value), prompt before " "deleting. With 'do', delete without prompting.", @@ -52,7 +53,7 @@ "'require' - data must pass validation before upload; " "'skip' - no validation is performed on data before upload; " "'ignore' - data is validated but upload proceeds regardless of validation results.", - type=click.Choice(list(UploadValidation)), + type=EnumChoice(UploadValidation), default="require", show_default=True, ) diff --git a/dandi/cli/tests/test_base.py b/dandi/cli/tests/test_base.py new file mode 100644 index 000000000..1576f422a --- /dev/null +++ b/dandi/cli/tests/test_base.py @@ -0,0 +1,74 @@ +from enum import Enum + +import click +from click.testing import CliRunner +import pytest + +from ..base import EnumChoice + + +class _Existing(str, Enum): + ERROR = "error" + SKIP = "skip" + OVERWRITE = "overwrite-different" + + def __str__(self) -> str: + return self.value + + +def _make_command(**option_kwargs): + @click.command() + @click.option("--existing", type=EnumChoice(_Existing), **option_kwargs) + def cmd(existing): + click.echo(f"{type(existing).__name__}:{existing!r}") + + return cmd + + +@pytest.mark.ai_generated +def test_enum_choice_accepts_member_value(): + captured = {} + + @click.command() + @click.option("--existing", type=EnumChoice(_Existing), default="error") + def cmd(existing): + captured["existing"] = existing + + r = CliRunner().invoke(cmd, ["--existing", "overwrite-different"]) + assert r.exit_code == 0, r.output + assert captured["existing"] is _Existing.OVERWRITE + + +@pytest.mark.ai_generated +def test_enum_choice_rejects_member_name(): + r = CliRunner().invoke(_make_command(default="error"), ["--existing", "SKIP"]) + assert r.exit_code != 0 + assert "'SKIP' is not one of" in r.output + + +@pytest.mark.ai_generated +def test_enum_choice_none_default_passes_through(): + captured = {} + + @click.command() + @click.option("--existing", type=EnumChoice(_Existing), default=None) + def cmd(existing): + captured["existing"] = existing + + r = CliRunner().invoke(cmd, []) + assert r.exit_code == 0, r.output + assert captured["existing"] is None + + +@pytest.mark.ai_generated +def test_enum_choice_string_default_converted_to_member(): + captured = {} + + @click.command() + @click.option("--existing", type=EnumChoice(_Existing), default="skip") + def cmd(existing): + captured["existing"] = existing + + r = CliRunner().invoke(cmd, []) + assert r.exit_code == 0, r.output + assert captured["existing"] is _Existing.SKIP diff --git a/pyproject.toml b/pyproject.toml index 7454bb12f..a7623e51c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,8 +46,7 @@ classifiers = [ dependencies = [ "bidsschematools ~= 1.0", "bids-validator-deno >= 2.0.5", - # >=8.2.0: https://github.com/pallets/click/issues/2911 - "click >= 7.1, <8.2.0", + "click >= 7.1", "click-didyoumean", "dandischema ~= 0.12.0", "etelemetry >= 0.2.2",