Enable type-checking

.github/workflows/typing.yml

@@ -0,0 +1,27 @@
+name: Type-check
+
+on:
+  - push
+  - pull_request
+
+jobs:
+  typing:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+
+      - name: Set up Python
+        uses: actions/setup-python@v2
+        with:
+          python-version: '3.7'
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install --upgrade tox
+
+      - name: Run type checker
+        run: tox -e typing

dandi/cli/base.py

@@ -120,4 +120,6 @@ def wrapper(obj, *args, **kwargs):
     return wrapper
 
 
-map_to_click_exceptions._do_map = not bool(os.environ.get("DANDI_DEVEL", None))
+map_to_click_exceptions._do_map = not bool(  # type: ignore[attr-defined]
+    os.environ.get("DANDI_DEVEL", None)
+)

dandi/consts.py

@@ -1,6 +1,6 @@
-from collections import namedtuple
 from enum import Enum
 import os
+from typing import NamedTuple, Optional
 
 #: A list of metadata fields which dandi extracts from .nwb files.
 #: Additional fields (such as ``number_of_*``) might be added by
@@ -87,7 +87,12 @@ class EmbargoStatus(Enum):
 dandiset_metadata_file = "dandiset.yaml"
 dandiset_identifier_regex = f"^{DANDISET_ID_REGEX}$"
 
-DandiInstance = namedtuple("DandiInstance", ("gui", "redirector", "api"))
+
+class DandiInstance(NamedTuple):
+    gui: Optional[str]
+    redirector: Optional[str]
+    api: Optional[str]
+
 
 # So it could be easily mapped to external IP (e.g. from within VM)
 # to test against instance running outside of current environment

dandi/dandiapi.py

@@ -569,7 +569,10 @@ def json_dict(self) -> Dict[str, Any]:
         Convert to a JSONable `dict`, omitting the ``client`` attribute and
         using the same field names as in the API
         """
-        return json.loads(self.json(exclude=self.JSON_EXCLUDE, by_alias=True))
+        return cast(
+            Dict[str, Any],
+            json.loads(self.json(exclude=self.JSON_EXCLUDE, by_alias=True)),
+        )
 
     class Config:
         allow_population_by_field_name = True
@@ -640,7 +643,7 @@ def __init__(
         client: DandiAPIClient,
         identifier: str,
         version: Union[str, Version, None] = None,
-        data: Optional[Dict[str, Any]] = None,
+        data: Union[Dict[str, Any], RemoteDandisetData, None] = None,
     ) -> None:
         #: The `DandiAPIClient` instance that returned this `RemoteDandiset`
         #: and which the latter will use for API requests
@@ -1076,9 +1079,12 @@ def upload_raw_asset(
         :param RemoteAsset replace_asset: If set, replace the given asset,
             which must have the same path as the new asset
         """
-        from .files import dandi_file
+        from .files import LocalAsset, dandi_file
 
-        return dandi_file(filepath).upload(
+        df = dandi_file(filepath)
+        if not isinstance(df, LocalAsset):
+            raise ValueError(f"{filepath}: not an asset file")
+        return df.upload(
             self, metadata=asset_metadata, jobs=jobs, replacing=replace_asset
         )
 
@@ -1114,9 +1120,12 @@ def iter_upload_raw_asset(
             ``"done"`` and an ``"asset"`` key containing the resulting
             `RemoteAsset`.
         """
-        from .files import dandi_file
+        from .files import LocalAsset, dandi_file
 
-        return dandi_file(filepath).iter_upload(
+        df = dandi_file(filepath)
+        if not isinstance(df, LocalAsset):
+            raise ValueError(f"{filepath}: not an asset file")
+        return df.iter_upload(
             self, metadata=asset_metadata, jobs=jobs, replacing=replace_asset
         )
 
@@ -1147,7 +1156,7 @@ class BaseRemoteAsset(APIBase):
     #: instead of performing an API call
     _metadata: Optional[Dict[str, Any]] = PrivateAttr(default_factory=None)
 
-    def __init__(self, **data: Any) -> None:
+    def __init__(self, **data: Any) -> None:  # type: ignore[no-redef]
         super().__init__(**data)
         # Pydantic insists on not initializing any attributes that start with
         # underscores, so we have to do it ourselves.
@@ -1167,7 +1176,7 @@ def from_metadata(
         This is a low-level method that non-developers would normally only use
         when acquiring data using means outside of this library.
         """
-        return BaseRemoteAsset(
+        return BaseRemoteAsset(  # type: ignore[call-arg]
             client=client,
             identifier=metadata["identifier"],
             path=metadata["path"],
@@ -1240,7 +1249,7 @@ def get_raw_digest(
             digest_type = digest_type.value
         metadata = self.get_raw_metadata()
         try:
-            return metadata["digest"][digest_type]
+            return cast(str, metadata["digest"][digest_type])
         except KeyError:
             raise NotFoundError(f"No {digest_type} digest found in metadata")
 
@@ -1277,6 +1286,7 @@ def get_content_url(
         If ``strip_query`` is true, any query parameters are removed from the
         final URL before returning it.
         """
+        url: str
         for url in self.get_raw_metadata().get("contentUrl", []):
             if re.search(regex, url):
                 break
@@ -1303,7 +1313,7 @@ def get_content_url(
 
     def get_download_file_iter(
         self, chunk_size: int = MAX_CHUNK_SIZE
-    ) -> Callable[..., Iterator[bytes]]:
+    ) -> Callable[[int], Iterator[bytes]]:
         """
         Returns a function that when called (optionally with an offset into the
         asset to start downloading at) returns a generator of chunks of the
@@ -1346,7 +1356,7 @@ def download(
         """
         downloader = self.get_download_file_iter(chunk_size=chunk_size)
         with open(filepath, "wb") as fp:
-            for chunk in downloader():
+            for chunk in downloader(0):
                 fp.write(chunk)
 
     @property
@@ -1413,6 +1423,7 @@ def from_data(
         This is a low-level method that non-developers would normally only use
         when acquiring data using means outside of this library.
         """
+        klass: Type[RemoteAsset]
         if data.get("blob") is not None:
             klass = RemoteBlobAsset
             if data.pop("zarr", None) is not None:
@@ -1423,7 +1434,7 @@ def from_data(
                 raise ValueError("Asset data contains both `blob` and `zarr`'")
         else:
             raise ValueError("Asset data contains neither `blob` nor `zarr`")
-        return klass(
+        return klass(  # type: ignore[call-arg]
             client=dandiset.client,
             dandiset_id=dandiset.identifier,
             version_id=dandiset.version_id,
@@ -1589,12 +1600,12 @@ class ZarrListing(BaseModel):
     @property
     def dirnames(self) -> List[str]:
         """The basenames of the directory URLs in `directories`"""
-        return [PurePosixPath(unquote(url.path)).name for url in self.directories]
+        return [PurePosixPath(unquote(url.path or "")).name for url in self.directories]
 
     @property
     def filenames(self) -> List[str]:
         """The basenames of the file URLs in `files`"""
-        return [PurePosixPath(unquote(url.path)).name for url in self.files]
+        return [PurePosixPath(unquote(url.path or "")).name for url in self.files]
 
 
 @dataclass
@@ -1794,7 +1805,7 @@ def get_listing(self) -> ZarrListing:
 
     def get_download_file_iter(
         self, chunk_size: int = MAX_CHUNK_SIZE
-    ) -> Callable[..., Iterator[bytes]]:
+    ) -> Callable[[int], Iterator[bytes]]:
         """
         Returns a function that when called (optionally with an offset into the
         file to start downloading at) returns a generator of chunks of the file

dandi/dandiarchive.py

@@ -25,11 +25,13 @@
 using the `navigate_url()` function.
 """
 
+from __future__ import annotations
+
 from abc import ABC, abstractmethod
 from contextlib import contextmanager
 import re
 from time import sleep
-from typing import Iterable, Iterator, Optional, Tuple
+from typing import Any, Dict, Iterable, Iterator, List, Optional, Tuple, cast
 from urllib.parse import unquote as urlunquote
 
 from pydantic import AnyHttpUrl, BaseModel, parse_obj_as, validator
@@ -72,7 +74,7 @@ class ParsedDandiURL(ABC, BaseModel):
 
     @validator("api_url")
     def _validate_api_url(cls, v: AnyHttpUrl) -> AnyHttpUrl:
-        return parse_obj_as(AnyHttpUrl, v.rstrip("/"))
+        return cast(AnyHttpUrl, parse_obj_as(AnyHttpUrl, v.rstrip("/")))
 
     def get_client(self) -> DandiAPIClient:
         """
@@ -178,9 +180,9 @@ def get_assets(
     ) -> Iterator[BaseRemoteAsset]:
         """Returns all assets in the Dandiset"""
         with _maybe_strict(strict):
-            yield from self.get_dandiset(client, lazy=not strict).get_assets(
-                order=order
-            )
+            d = self.get_dandiset(client, lazy=not strict)
+            assert d is not None
+            yield from d.get_assets(order=order)
 
 
 class SingleAssetURL(ParsedDandiURL):
@@ -256,7 +258,9 @@ def get_assets(
         nothing is yielded if ``strict`` is false.
         """
         with _maybe_strict(strict):
-            yield self.get_dandiset(client, lazy=not strict).get_asset(self.asset_id)
+            d = self.get_dandiset(client, lazy=not strict)
+            assert d is not None
+            yield d.get_asset(self.asset_id)
 
     def get_asset_ids(self, client: DandiAPIClient) -> Iterator[str]:
         """Yields the ID of the asset (regardless of whether it exists)"""
@@ -273,9 +277,9 @@ def get_assets(
     ) -> Iterator[BaseRemoteAsset]:
         """Returns the assets whose paths start with `path`"""
         with _maybe_strict(strict):
-            yield from self.get_dandiset(
-                client, lazy=not strict
-            ).get_assets_with_path_prefix(self.path, order=order)
+            d = self.get_dandiset(client, lazy=not strict)
+            assert d is not None
+            yield from d.get_assets_with_path_prefix(self.path, order=order)
 
 
 class AssetItemURL(SingleAssetURL):
@@ -297,6 +301,7 @@ def get_assets(
         """
         try:
             dandiset = self.get_dandiset(client, lazy=not strict)
+            assert dandiset is not None
             # Force evaluation of the version here instead of when
             # get_asset_by_path() is called so we don't get nonexistent
             # dandisets with unspecified versions mixed up with nonexistent
@@ -343,9 +348,9 @@ def get_assets(
         if not path.endswith("/"):
             path += "/"
         with _maybe_strict(strict):
-            yield from self.get_dandiset(
-                client, lazy=not strict
-            ).get_assets_with_path_prefix(path, order=order)
+            d = self.get_dandiset(client, lazy=not strict)
+            assert d is not None
+            yield from d.get_assets_with_path_prefix(path, order=order)
 
 
 @contextmanager
@@ -403,7 +408,7 @@ class _dandi_url_parser:
     dandiset_id_grp = f"(?P<dandiset_id>{DANDISET_ID_REGEX})"
     # Should absorb port and "api/":
     server_grp = "(?P<server>(?P<protocol>https?)://(?P<hostname>[^/]+)/(api/)?)"
-    known_urls = [
+    known_urls: List[Tuple[re.Pattern[str], Dict[str, Any], str]] = [
         # List of (regex, settings, display string) triples
         #
         # Settings:

dandi/delete.py

@@ -7,7 +7,7 @@
 
 from .consts import DRAFT, dandiset_metadata_file
 from .dandiapi import DandiAPIClient, RemoteAsset, RemoteDandiset
-from .dandiarchive import DandisetURL, ParsedDandiURL, parse_dandi_url
+from .dandiarchive import BaseAssetIDURL, DandisetURL, ParsedDandiURL, parse_dandi_url
 from .exceptions import NotFoundError
 from .utils import get_instance, is_url
 
@@ -47,8 +47,10 @@ def set_dandiset(self, api_url: str, dandiset_id: str) -> bool:
                     raise
         elif not is_same_url(self.client.api_url, api_url):
             raise ValueError("Cannot delete assets from multiple API instances at once")
-        elif self.dandiset.identifier != dandiset_id:
-            raise ValueError("Cannot delete assets from multiple Dandisets at once")
+        else:
+            assert self.dandiset is not None
+            if self.dandiset.identifier != dandiset_id:
+                raise ValueError("Cannot delete assets from multiple Dandisets at once")
         return True
 
     def add_asset(self, asset: RemoteAsset) -> None:
@@ -67,6 +69,7 @@ def register_asset(
     ) -> None:
         if not self.set_dandiset(api_url, dandiset_id):
             return
+        assert self.dandiset is not None
         try:
             asset = self.dandiset.get_asset_by_path(asset_path)
         except NotFoundError:
@@ -84,6 +87,7 @@ def register_asset_folder(
         if not self.set_dandiset(api_url, dandiset_id):
             return
         any_assets = False
+        assert self.dandiset is not None
         for asset in self.dandiset.get_assets_with_path_prefix(folder_path):
             self.add_asset(asset)
             any_assets = True
@@ -93,10 +97,15 @@ def register_asset_folder(
             )
 
     def register_assets_url(self, url: str, parsed_url: ParsedDandiURL) -> None:
+        if isinstance(parsed_url, BaseAssetIDURL):
+            raise ValueError("Cannot delete an asset identified by just an ID")
+        assert parsed_url.dandiset_id is not None
         if not self.set_dandiset(parsed_url.api_url, parsed_url.dandiset_id):
             return
         any_assets = False
+        assert self.client is not None
         for a in parsed_url.get_assets(self.client):
+            assert isinstance(a, RemoteAsset)
             self.add_asset(a)
             any_assets = True
         if not any_assets and not self.skip_missing:
@@ -110,6 +119,7 @@ def register_url(self, url: str) -> None:
                     "Dandi API server does not support deletion of individual"
                     " versions of a dandiset"
                 )
+            assert parsed_url.dandiset_id is not None
             self.register_dandiset(parsed_url.api_url, parsed_url.dandiset_id)
         else:
             if parsed_url.version_id is None:
@@ -128,6 +138,8 @@ def register_local_path_equivalent(self, instance_name: str, filepath: str) -> N
             self.register_asset(api_url, dandiset_id, DRAFT, asset_path)
 
     def confirm(self) -> bool:
+        if self.dandiset is None:
+            raise ValueError("confirm() called before registering anything to delete")
         if self.deleting_dandiset:
             msg = f"Delete Dandiset {self.dandiset.identifier}?"
         else:
@@ -139,6 +151,7 @@ def confirm(self) -> bool:
 
     def delete_dandiset(self) -> None:
         if self.deleting_dandiset:
+            assert self.dandiset is not None
             self.dandiset.delete()
         else:
             raise RuntimeError(
@@ -196,8 +209,8 @@ def delete(
                 style=pyout_style, columns=rec_fields, max_workers=jobs
             )
             with out:
-                for gen in deleter.process_assets_pyout():
-                    out(gen)
+                for r in deleter.process_assets_pyout():
+                    out(r)
 
 
 def find_local_asset(filepath: str) -> Tuple[str, str]: