Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace magic string arguments with enums #1357

Merged
merged 4 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion dandi/cli/cmd_digest.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import click

from .base import map_to_click_exceptions
Expand All @@ -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

Expand Down
33 changes: 18 additions & 15 deletions dandi/cli/cmd_download.py
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -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.",
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dandi/cli/cmd_ls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 5 additions & 4 deletions dandi/cli/cmd_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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,
Expand All @@ -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;"
Expand All @@ -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:
Expand Down
29 changes: 15 additions & 14 deletions dandi/cli/cmd_organize.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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",
)
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dandi/cli/cmd_service_scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
32 changes: 18 additions & 14 deletions dandi/cli/cmd_upload.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import click

from .base import (
Expand All @@ -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 "
Expand All @@ -24,6 +27,7 @@
@click.option(
"-J",
"--jobs",
"jobs_pair",
type=IntColonInt(),
help=(
"Number of assets to upload in parallel and, optionally, number of"
Expand All @@ -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,
)
Expand All @@ -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.

Expand All @@ -89,11 +93,11 @@
"""
from ..upload import upload

if jobs is None:
if jobs_pair is None:

Check warning on line 96 in dandi/cli/cmd_upload.py

View check run for this annotation

Codecov / codecov/patch

dandi/cli/cmd_upload.py#L96

Added line #L96 was not covered by tests
jobs = None
jobs_per_file = None
else:
jobs, jobs_per_file = jobs
jobs, jobs_per_file = jobs_pair

Check warning on line 100 in dandi/cli/cmd_upload.py

View check run for this annotation

Codecov / codecov/patch

dandi/cli/cmd_upload.py#L100

Added line #L100 was not covered by tests

upload(
paths,
Expand Down
Loading