Merge pull request #1357 from dandi/more-enums

Replace magic string arguments with enums

dandi/cli/cmd_digest.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import click
 
 from .base import map_to_click_exceptions
@@ -18,7 +20,7 @@
 )
 @click.argument("paths", nargs=-1, type=click.Path(exists=True))
 @map_to_click_exceptions
-def digest(paths, digest_alg):
+def digest(paths: tuple[str, ...], digest_alg: str) -> None:
     """Calculate file digests"""
     from ..support.digests import get_digest
 

dandi/cli/cmd_download.py

@@ -1,9 +1,13 @@
+from __future__ import annotations
+
+from collections.abc import Sequence
 import os
 
 import click
 
 from .base import ChoiceList, IntColonInt, instance_option, map_to_click_exceptions
 from ..dandiarchive import _dandi_url_parser, parse_dandi_url
+from ..download import DownloadExisting, DownloadFormat, PathType
 from ..utils import get_instance
 
 
@@ -28,9 +32,8 @@
 @click.option(
     "-e",
     "--existing",
-    type=click.Choice(
-        ["error", "skip", "overwrite", "overwrite-different", "refresh"]
-    ),  # TODO: verify-reupload (to become default)
+    type=click.Choice(list(DownloadExisting)),
+    # TODO: verify-reupload (to become default)
     help="What to do if a file found existing locally. 'refresh': verify "
     "that according to the size and mtime, it is the same file, if not - "
     "download and overwrite.",
@@ -41,12 +44,12 @@
     "-f",
     "--format",
     help="Choose the format/frontend for output. TODO: support all of the ls",
-    type=click.Choice(["pyout", "debug"]),
+    type=click.Choice(list(DownloadFormat)),
     default="pyout",
 )
 @click.option(
     "--path-type",
-    type=click.Choice(["exact", "glob"]),
+    type=click.Choice(list(PathType)),
     default="exact",
     help="Whether to interpret asset paths in URLs as exact matches or glob patterns",
     show_default=True,
@@ -96,16 +99,16 @@
 @click.argument("url", nargs=-1)
 @map_to_click_exceptions
 def download(
-    url,
-    output_dir,
-    existing,
-    jobs,
-    format,
-    download_types,
-    sync,
-    dandi_instance,
-    path_type,
-):
+    url: Sequence[str],
+    output_dir: str,
+    existing: DownloadExisting,
+    jobs: tuple[int, int],
+    format: DownloadFormat,
+    download_types: set[str],
+    sync: bool,
+    dandi_instance: str,
+    path_type: PathType,
+) -> None:
     # We need to import the download module rather than the download function
     # so that the tests can properly patch the function with a mock.
     from .. import download

dandi/cli/cmd_ls.py

@@ -331,7 +331,7 @@ def get_metadata_ls(
     path, keys, errors, flatten=False, schema=None, use_fake_digest=False
 ):
     from ..dandiset import Dandiset
-    from ..metadata import get_metadata, nwb2asset
+    from ..metadata.nwb import get_metadata, nwb2asset
     from ..pynwb_utils import get_nwb_version, ignore_benign_pynwb_warnings
     from ..support.digests import get_digest
 

dandi/cli/cmd_move.py

@@ -3,6 +3,7 @@
 import click
 
 from .base import devel_debug_option, instance_option, map_to_click_exceptions
+from ..move import MoveExisting, MoveWorkOn
 
 
 @click.command()
@@ -15,7 +16,7 @@
 @click.option(
     "-e",
     "--existing",
-    type=click.Choice(["error", "skip", "overwrite"]),
+    type=click.Choice(list(MoveExisting)),
     default="error",
     help="How to handle assets that would be moved to a destination that already exists",
     show_default=True,
@@ -29,7 +30,7 @@
 @click.option(
     "-w",
     "--work-on",
-    type=click.Choice(["auto", "both", "local", "remote"]),
+    type=click.Choice(list(MoveWorkOn)),
     default="auto",
     help=(
         "Whether to operate on the local Dandiset, remote Dandiset, or both;"
@@ -47,10 +48,10 @@ def move(
     paths: tuple[str, ...],
     dandiset: str | None,
     dry_run: bool,
-    existing: str,
+    existing: MoveExisting,
     jobs: int | None,
     regex: bool,
-    work_on: str,
+    work_on: MoveWorkOn,
     dandi_instance: str,
     devel_debug: bool = False,
 ) -> None:

dandi/cli/cmd_organize.py

@@ -3,7 +3,8 @@
 import click
 
 from .base import dandiset_path_option, devel_debug_option, map_to_click_exceptions
-from ..consts import dandi_layout_fields, file_operation_modes
+from ..consts import dandi_layout_fields
+from ..organize import CopyMode, FileOperationMode, OrganizeInvalid
 
 
 @click.command()
@@ -16,7 +17,7 @@
 @click.option(
     "--invalid",
     help="What to do if files without sufficient metadata are encountered.",
-    type=click.Choice(["fail", "warn"]),
+    type=click.Choice(list(OrganizeInvalid)),
     default="fail",
     show_default=True,
 )
@@ -29,7 +30,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(file_operation_modes),
+    type=click.Choice(list(FileOperationMode)),
     default="auto",
     show_default=True,
 )
@@ -44,7 +45,7 @@
 )
 @click.option(
     "--media-files-mode",
-    type=click.Choice(["copy", "move", "symlink", "hardlink"]),
+    type=click.Choice(list(CopyMode)),
     default=None,
     help="How to relocate video files referenced by NWB files",
 )
@@ -63,16 +64,16 @@
 @devel_debug_option()
 @map_to_click_exceptions
 def organize(
-    paths,
-    required_fields,
-    dandiset_path=None,
-    invalid="fail",
-    files_mode="auto",
-    devel_debug=False,
-    update_external_file_paths=False,
-    media_files_mode=None,
-    jobs=None,
-):
+    paths: tuple[str, ...],
+    required_fields: tuple[str, ...],
+    dandiset_path: str | None,
+    invalid: OrganizeInvalid,
+    files_mode: FileOperationMode,
+    media_files_mode: CopyMode | None,
+    update_external_file_paths: bool,
+    jobs: int | None,
+    devel_debug: bool = False,
+) -> None:
     """(Re)organize files according to the metadata.
 
     The purpose of this command is to take advantage of metadata contained in

dandi/cli/cmd_service_scripts.py

@@ -47,7 +47,7 @@ def reextract_metadata(url: str, diff: bool, when: str) -> None:
     Running this command requires the fsspec library to be installed with the
     `http` extra (e.g., `pip install "fsspec[http]"`).
     """
-    from ..metadata import nwb2asset  # Avoid heavy import at top level
+    from ..metadata.nwb import nwb2asset  # Avoid heavy import at top level
 
     parsed_url = parse_dandi_url(url)
     if parsed_url.dandiset_id is None:

dandi/cli/cmd_upload.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import click
 
 from .base import (
@@ -7,13 +9,14 @@
     instance_option,
     map_to_click_exceptions,
 )
+from ..upload import UploadExisting, UploadValidation
 
 
 @click.command()
 @click.option(
     "-e",
     "--existing",
-    type=click.Choice(["error", "skip", "force", "overwrite", "refresh"]),
+    type=click.Choice(list(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 "
@@ -24,6 +27,7 @@
 @click.option(
     "-J",
     "--jobs",
+    "jobs_pair",
     type=IntColonInt(),
     help=(
         "Number of assets to upload in parallel and, optionally, number of"
@@ -36,7 +40,7 @@
 @click.option(
     "--validation",
     help="Data must pass validation before the upload.  Use of this option is highly discouraged.",
-    type=click.Choice(["require", "skip", "ignore"]),
+    type=click.Choice(list(UploadValidation)),
     default="require",
     show_default=True,
 )
@@ -61,17 +65,17 @@
 @devel_debug_option()
 @map_to_click_exceptions
 def upload(
-    paths,
-    jobs,
-    sync,
-    dandi_instance,
-    existing="refresh",
-    validation="require",
+    paths: tuple[str, ...],
+    jobs_pair: tuple[int, int] | None,
+    sync: bool,
+    dandi_instance: str,
+    existing: UploadExisting,
+    validation: UploadValidation,
     # Development options should come as kwargs
-    allow_any_path=False,
-    upload_dandiset_metadata=False,
-    devel_debug=False,
-):
+    allow_any_path: bool = False,
+    upload_dandiset_metadata: bool = False,
+    devel_debug: bool = False,
+) -> None:
     """
     Upload Dandiset files to DANDI Archive.
 
@@ -89,11 +93,11 @@ def upload(
     """
     from ..upload import upload
 
-    if jobs is None:
+    if jobs_pair is None:
         jobs = None
         jobs_per_file = None
     else:
-        jobs, jobs_per_file = jobs
+        jobs, jobs_per_file = jobs_pair
 
     upload(
         paths,