mypy: strict typings (#221)

* mypy: strict typings

* update pre-commit hooks and mypy

Author: skshetry

.pre-commit-config.yaml

@@ -2,7 +2,7 @@ default_language_version:
   python: python3
 repos:
   - repo: https://github.com/psf/black
-    rev: 23.1.0
+    rev: 23.7.0
     hooks:
       - id: black
   - repo: https://github.com/pre-commit/pre-commit-hooks
@@ -24,12 +24,12 @@ repos:
       - id: sort-simple-yaml
       - id: trailing-whitespace
   - repo: https://github.com/codespell-project/codespell
-    rev: v2.2.2
+    rev: v2.2.5
     hooks:
       - id: codespell
         additional_dependencies: ["tomli"]
   - repo: https://github.com/asottile/pyupgrade
-    rev: v3.3.1
+    rev: v3.10.1
     hooks:
       - id: pyupgrade
         args: [--py38-plus]
@@ -38,7 +38,7 @@ repos:
     hooks:
       - id: isort
   - repo: https://github.com/pycqa/flake8
-    rev: 6.0.0
+    rev: 6.1.0
     hooks:
       - id: flake8
         additional_dependencies:
@@ -47,8 +47,8 @@ repos:
           - flake8-debugger==4.1.2
           - flake8-string-format==0.3.0
   - repo: https://github.com/pycqa/bandit
-    rev: 1.7.4
+    rev: 1.7.5
     hooks:
       - id: bandit
         args: [-c, pyproject.toml]
-        additional_dependencies: ["toml"]
+        additional_dependencies: [".[toml]"]

pyproject.toml

@@ -56,14 +56,23 @@ show_error_codes = true
 show_error_context = true
 show_traceback = true
 pretty = true
-check_untyped_defs = false
+check_untyped_defs = true
 # Warnings
 warn_no_return = true
 warn_redundant_casts = true
 warn_unreachable = true
-ignore_missing_imports = true
+strict_equality = true
+no_implicit_optional = true
+warn_unused_configs = true
 files = ["src", "tests"]
 
+[[tool.mypy.overrides]]
+ignore_missing_imports = true
+module = [
+    "fsspec.*",
+    "funcy",
+]
+
 [tool.pylint.format]
 max-line-length = 88
 

setup.cfg

@@ -38,10 +38,11 @@ tests =
     pytest-cov==4.0.0
     pytest-mock==3.8.2
     pylint==2.15.0
-    mypy==0.971
+    mypy==1.5.1
     pytest-servers[s3]==0.1.3
 dev =
     %(tests)s
+    types-tqdm
 
 [options.packages.find]
 exclude =

src/dvc_objects/_tqdm.py

@@ -144,7 +144,11 @@ def format_dict(self):
         d = super().format_dict
         ncols = d["ncols"] or 80
         # assumes `bar_format` has max one of ("ncols_desc" & "ncols_info")
-        ncols_left = ncols - len(self.format_meter(ncols_desc=1, ncols_info=1, **d)) + 1
+
+        meter = self.format_meter(  # type: ignore[call-arg]
+            ncols_desc=1, ncols_info=1, **d
+        )
+        ncols_left = ncols - len(meter) + 1
         ncols_left = max(ncols_left, 0)
         if ncols_left:
             d["ncols_desc"] = d["ncols_info"] = ncols_left

src/dvc_objects/db.py

@@ -350,12 +350,12 @@ def all(self, jobs=None):
         remote_size, remote_oids = self._estimate_remote_size()
         return self._list_oids_traverse(remote_size, remote_oids, jobs=jobs)
 
-    def list_oids_exists(self, oids: List[str], jobs: Optional[int] = None):
+    def list_oids_exists(self, oids: Iterable[str], jobs: Optional[int] = None):
         """Return list of the specified oids which exist in this fs.
         Hashes will be queried individually.
         """
-        logger.debug(f"Querying {len(oids)} oids via object_exists")
         paths = list(map(self.oid_to_path, oids))
+        logger.debug(f"Querying {len(paths)} oids via object_exists")
         in_remote = self.fs.exists(paths, batch_size=jobs)
         yield from itertools.compress(oids, in_remote)
 

src/dvc_objects/executors.py

@@ -28,7 +28,7 @@ class ThreadPoolExecutor(futures.ThreadPoolExecutor):
     _max_workers: int
 
     def __init__(
-        self, max_workers: int = None, cancel_on_error: bool = False, **kwargs
+        self, max_workers: Optional[int] = None, cancel_on_error: bool = False, **kwargs
     ):
         super().__init__(max_workers=max_workers, **kwargs)
         self._cancel_on_error = cancel_on_error
@@ -62,26 +62,26 @@ def shutdown(  # pylint: disable=arguments-differ
         if sys.version_info > (3, 9):
             # pylint: disable=unexpected-keyword-arg
             return super().shutdown(wait=wait, cancel_futures=cancel_futures)
-
-        with self._shutdown_lock:
-            self._shutdown = True
-            if cancel_futures:
-                # Drain all work items from the queue, and then cancel their
-                # associated futures.
-                while True:
-                    try:
-                        work_item = self._work_queue.get_nowait()
-                    except queue.Empty:
-                        break
-                    if work_item is not None:
-                        work_item.future.cancel()
-
-            # Send a wake-up to prevent threads calling
-            # _work_queue.get(block=True) from permanently blocking.
-            self._work_queue.put(None)
-        if wait:
-            for t in self._threads:
-                t.join()
+        else:
+            with self._shutdown_lock:
+                self._shutdown = True
+                if cancel_futures:
+                    # Drain all work items from the queue, and then cancel their
+                    # associated futures.
+                    while True:
+                        try:
+                            work_item = self._work_queue.get_nowait()
+                        except queue.Empty:
+                            break
+                        if work_item is not None:
+                            work_item.future.cancel()
+
+                # Send a wake-up to prevent threads calling
+                # _work_queue.get(block=True) from permanently blocking.
+                self._work_queue.put(None)  # type: ignore[arg-type]
+            if wait:
+                for t in self._threads:
+                    t.join()
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         if self._cancel_on_error:

src/dvc_objects/fs/base.py

@@ -22,7 +22,9 @@
 )
 
 from fsspec.asyn import get_loop
-from funcy import cached_property, once_per_args
+from funcy import once_per_args
+
+from dvc_objects.utils import cached_property
 
 from ..executors import ThreadPoolExecutor, batch_coros
 from .callbacks import DEFAULT_CALLBACK, Callback
@@ -230,7 +232,7 @@ def read_block(
         path: AnyFSPath,
         offset: int,
         length: int,
-        delimiter: bytes = None,
+        delimiter: Optional[bytes] = None,
     ) -> bytes:
         return self.fs.read_block(path, offset, length, delimiter=delimiter)
 
@@ -248,16 +250,16 @@ def cat_ranges(
         paths: List[AnyFSPath],
         starts: List[int],
         ends: List[int],
-        max_gap: int = None,
+        max_gap: Optional[int] = None,
         **kwargs,
     ) -> List[bytes]:
         return self.fs.cat_ranges(paths, starts, ends, max_gap=max_gap, **kwargs)
 
     def cat_file(
         self,
         path: AnyFSPath,
-        start: int = None,
-        end: int = None,
+        start: Optional[int] = None,
+        end: Optional[int] = None,
         **kwargs: Any,
     ) -> bytes:
         return self.fs.cat_file(path, start=start, end=end, **kwargs)
@@ -277,9 +279,9 @@ def pipe_file(self, path: AnyFSPath, value: bytes, **kwargs: Any) -> None:
     def read_text(
         self,
         path: AnyFSPath,
-        encoding: str = None,
-        errors: str = None,
-        newline: str = None,
+        encoding: Optional[str] = None,
+        errors: Optional[str] = None,
+        newline: Optional[str] = None,
         **kwargs: Any,
     ) -> str:
         return self.fs.read_text(
@@ -290,9 +292,9 @@ def write_text(
         self,
         path: AnyFSPath,
         value: str,
-        encoding: str = None,
-        errors: str = None,
-        newline: str = None,
+        encoding: Optional[str] = None,
+        errors: Optional[str] = None,
+        newline: Optional[str] = None,
         **kwargs: Any,
     ) -> None:
         self.fs.write_text(
@@ -531,7 +533,7 @@ def put_file(
         from_file: Union[AnyFSPath, "BinaryIO"],
         to_info: AnyFSPath,
         callback: Callback = DEFAULT_CALLBACK,
-        size: int = None,
+        size: Optional[int] = None,
         **kwargs,
     ) -> None:
         if size:
@@ -578,7 +580,7 @@ def du(
         self,
         path: AnyFSPath,
         total: bool = True,
-        maxdepth: int = None,
+        maxdepth: Optional[int] = None,
         **kwargs: Any,
     ) -> Union[int, Dict[AnyFSPath, int]]:
         return self.fs.du(path, total=total, maxdepth=maxdepth, **kwargs)
@@ -589,7 +591,7 @@ def put(
         to_info: Union[AnyFSPath, List[AnyFSPath]],
         callback: "Callback" = DEFAULT_CALLBACK,
         recursive: bool = False,  # pylint: disable=unused-argument
-        batch_size: int = None,
+        batch_size: Optional[int] = None,
     ):
         jobs = batch_size or self.jobs
         if self.fs.async_impl:
@@ -617,7 +619,7 @@ def get(
         to_info: Union[AnyFSPath, List[AnyFSPath]],
         callback: "Callback" = DEFAULT_CALLBACK,
         recursive: bool = False,  # pylint: disable=unused-argument
-        batch_size: int = None,
+        batch_size: Optional[int] = None,
     ) -> None:
         # Currently, the implementation is non-recursive if the paths are
         # provided as a list, and recursive if it's a single path.

src/dvc_objects/fs/callbacks.py

@@ -1,9 +1,10 @@
 from contextlib import ExitStack
 from functools import wraps
-from typing import TYPE_CHECKING, Any, Dict, Optional, TypeVar, overload
+from typing import TYPE_CHECKING, Any, Dict, Optional, TypeVar, cast, overload
 
 import fsspec
-from funcy import cached_property
+
+from dvc_objects.utils import cached_property
 
 if TYPE_CHECKING:
     from typing import Awaitable, BinaryIO, Callable, TextIO, Union
@@ -33,7 +34,7 @@ def wrap_attr(
         from tqdm.utils import CallbackIOWrapper
 
         wrapped = CallbackIOWrapper(self.relative_update, fobj, method)
-        return wrapped
+        return cast("Union[TextIO, BinaryIO]", wrapped)
 
     def wrap_fn(self, fn: "Callable[_P, _R]") -> "Callable[_P, _R]":
         @wraps(fn)
@@ -121,7 +122,7 @@ def branch(  # pylint: disable=arguments-differ
         path_1: "Union[str, BinaryIO]",
         path_2: str,
         kwargs: Dict[str, Any],
-        child: "Callback" = None,
+        child: Optional["Callback"] = None,
     ) -> "Callback":
         child = kwargs["callback"] = child or DEFAULT_CALLBACK
         return child
@@ -136,7 +137,7 @@ def __init__(
         self,
         size: Optional[int] = None,
         value: int = 0,
-        progress_bar: "Tqdm" = None,
+        progress_bar: Optional["Tqdm"] = None,
         **tqdm_kwargs,
     ):
         tqdm_kwargs["total"] = size or -1

src/dvc_objects/fs/errors.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING, List, Optional
 
 if TYPE_CHECKING:
     from .base import FileSystem
@@ -18,7 +18,7 @@ def __init__(
         fs: "FileSystem",
         protocol: str,
         url: str,
-        missing: List[str] = None,
+        missing: Optional[List[str]] = None,
     ) -> None:
         self.protocol = protocol
         self.fs = fs

src/dvc_objects/fs/local.py

@@ -4,7 +4,9 @@
 import threading
 
 import fsspec
-from funcy import cached_property, wrap_prop
+from funcy import wrap_prop
+
+from dvc_objects.utils import cached_property
 
 from . import system
 from .base import FileSystem
@@ -183,7 +185,7 @@ class LocalFileSystem(FileSystem):
     PARAM_PATH = "path"
     TRAVERSE_PREFIX_LEN = 2
 
-    @wrap_prop(threading.Lock())
+    @wrap_prop(threading.Lock())  # type: ignore[misc]
     @cached_property
     def fs(self):
         return FsspecLocalFileSystem(**self.config)

src/dvc_objects/fs/utils.py

@@ -104,7 +104,7 @@ def remove(path: "AnyFSPath") -> None:
             raise
 
 
-def makedirs(path, exist_ok: bool = False, mode: int = None) -> None:
+def makedirs(path, exist_ok: bool = False, mode: Optional[int] = None) -> None:
     if mode is None:
         os.makedirs(path, exist_ok=exist_ok)
         return
@@ -144,9 +144,9 @@ def makedirs(path, exist_ok: bool = False, mode: int = None) -> None:
 def copyfile(
     src: "AnyFSPath",
     dest: "AnyFSPath",
-    callback: "Callback" = None,
+    callback: Optional["Callback"] = None,
     no_progress_bar: bool = False,
-    name: str = None,
+    name: Optional[str] = None,
 ) -> None:
     """Copy file with progress bar"""
     name = name if name else os.path.basename(dest)

src/dvc_objects/transfer.py

@@ -1,11 +1,11 @@
-from typing import TYPE_CHECKING, Set
+from typing import TYPE_CHECKING, Optional, Set
 
 if TYPE_CHECKING:
     from .db import ObjectDB
 
 
 def transfer(
-    src: "ObjectDB", dest: "ObjectDB", oids: Set["str"], jobs: int = None
+    src: "ObjectDB", dest: "ObjectDB", oids: Set["str"], jobs: Optional[int] = None
 ) -> Set["str"]:
     src_exists = set(src.oids_exist(oids, jobs=jobs))
     src_missing = oids - src_exists

src/dvc_objects/utils.py

@@ -0,0 +1,8 @@
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from functools import cached_property
+else:
+    from funcy import cached_property  # noqa: TID251
+
+__all__ = ["cached_property"]