Merge pull request #220 from openzim/strict_mode

Activate strict mode and fix type hints

Author: benoit74

.pre-commit-config.yaml

@@ -16,7 +16,7 @@ repos:
     hooks:
       - id: ruff
   - repo: https://github.com/RobertCraigie/pyright-python
-    rev: v1.1.385
+    rev: v1.1.391
     hooks:
       - id: pyright
         name: pyright (system)

contrib/encode_video.py

@@ -1,5 +1,3 @@
-from __future__ import annotations
-
 import sys
 from pathlib import Path
 
@@ -20,7 +18,6 @@ def encode_video(src_path: Path, dst_path: Path, preset: str):
         src_path=src_path,
         dst_path=dst_path,
         ffmpeg_args=preset_cls().to_ffmpeg_args(),
-        with_process=True,
     )  # pyright: ignore[reportGeneralTypeIssues] (returned type is variable, depending on `with_process` value)
     if not success:
         logger.error(f"conversion failed:\n{process.stdout}")

pyproject.toml

@@ -56,7 +56,7 @@ lint = [
   "ruff==0.8.2",
 ]
 check = [
-  "pyright==1.1.390",
+  "pyright==1.1.391",
   "pytest==8.3.4",
 ]
 test = [
@@ -279,19 +279,17 @@ exclude_lines = [
   "if __name__ == .__main__.:",
   "if TYPE_CHECKING:",
   "class .*Protocol.*",
+  "@abstractmethod",
 ]
 
 [tool.pyright]
 include = ["contrib", "src", "tests", "tasks.py"]
 exclude = [".env/**", ".venv/**"]
 extraPaths = ["src"]
 pythonVersion = "3.12"
-typeCheckingMode="basic"
+typeCheckingMode="strict"
 disableBytesTypePromotions = true
 
-[tool.pyright.overrides]
-strict = true  # Enable strict mode for specific files
-
 [[tool.pyright.overrides.files]]
 files = [
   "src/zimscraperlib/rewriting**/*.py",

rules/generate_rules.py

@@ -156,11 +156,11 @@
 {% endfor %}
     ]
 )
-def {{ rule['name'] }}_case(request):
+def {{ rule['name'] }}_case(request: pytest.FixtureRequest):
     yield request.param
 
 
-def test_fuzzyrules_{{ rule['name'] }}({{ rule['name'] }}_case):
+def test_fuzzyrules_{{ rule['name'] }}({{ rule['name'] }}_case: ContentForTests):
     assert (
         ArticleUrlRewriter.apply_additional_rules({{ rule['name'] }}_case.input_str)
         == {{ rule['name'] }}_case.expected_str

src/zimscraperlib/__init__.py

@@ -1,6 +1,3 @@
-#!/usr/bin/env python
-# vim: ai ts=4 sts=4 et sw=4 nu
-
 import logging as stdlogging
 import os
 

src/zimscraperlib/constants.py

@@ -1,6 +1,3 @@
-#!/usr/bin/env python3
-# vim: ai ts=4 sts=4 et sw=4 nu
-
 import pathlib
 import re
 

src/zimscraperlib/download.py

@@ -1,18 +1,13 @@
-#!/usr/bin/env python3
-# vim: ai ts=4 sts=4 et sw=4 nu
-
-from __future__ import annotations
-
 import pathlib
 import subprocess
 from concurrent.futures import Future, ThreadPoolExecutor
-from typing import ClassVar
+from typing import Any, ClassVar
 
 import requests
 import requests.adapters
 import requests.structures
 import urllib3.util
-import yt_dlp as youtube_dl
+import yt_dlp as youtube_dl  # pyright: ignore[reportMissingTypeStubs]
 
 from zimscraperlib import logger
 from zimscraperlib.constants import DEFAULT_WEB_REQUESTS_TIMEOUT
@@ -31,24 +26,24 @@ def __init__(self, threads: int | None = 1) -> None:
     def __enter__(self):
         return self
 
-    def __exit__(self, *args):
+    def __exit__(self, *_: Any):
         self.shutdown()
 
     def shutdown(self) -> None:
         """shuts down the executor, awaiting completion"""
         self.executor.shutdown(wait=True)
 
-    def _run_youtube_dl(self, url: str, options: dict) -> None:
+    def _run_youtube_dl(self, url: str, options: dict[str, Any]) -> None:
         with youtube_dl.YoutubeDL(options) as ydl:
-            ydl.download([url])
+            ydl.download([url])  # pyright: ignore[reportUnknownMemberType]
 
     def download(
         self,
         url: str,
-        options: dict | None,
+        options: dict[str, Any] | None,
         *,
         wait: bool | None = True,
-    ) -> bool | Future:
+    ) -> bool | Future[Any]:
         """Downloads video using initialized executor.
 
         url: URL or Video ID
@@ -66,7 +61,7 @@ def download(
         return True
 
 
-class YoutubeConfig(dict):
+class YoutubeConfig(dict[str, str | bool | int | None]):
     options: ClassVar[dict[str, str | bool | int | None]] = {}
     defaults: ClassVar[dict[str, str | bool | int | None]] = {
         "writethumbnail": True,
@@ -82,7 +77,7 @@ class YoutubeConfig(dict):
         "outtmpl": "video.%(ext)s",
     }
 
-    def __init__(self, **kwargs):
+    def __init__(self, **kwargs: str | bool | int | None):
         super().__init__(self, **type(self).defaults)
         self.update(self.options)
         self.update(kwargs)
@@ -92,7 +87,7 @@ def get_options(
         cls,
         target_dir: pathlib.Path | None = None,
         filepath: pathlib.Path | None = None,
-        **options,
+        **options: str | bool | int | None,
     ):
         if "outtmpl" not in options:
             outtmpl = cls.options.get("outtmpl", cls.defaults["outtmpl"])
@@ -143,9 +138,10 @@ def save_large_file(url: str, fpath: pathlib.Path) -> None:
     )
 
 
-def _get_retry_adapter(
+def get_retry_adapter(
     max_retries: int | None = 5,
 ) -> requests.adapters.BaseAdapter:
+    """A requests adapter to automatically retry on known HTTP status that can be"""
     retries = urllib3.util.retry.Retry(
         total=max_retries,  # total number of retries
         connect=max_retries,  # connection errors
@@ -169,7 +165,7 @@ def _get_retry_adapter(
 def get_session(max_retries: int | None = 5) -> requests.Session:
     """Session to hold cookies and connection pool together"""
     session = requests.Session()
-    session.mount("http", _get_retry_adapter(max_retries))  # tied to http and https
+    session.mount("http", get_retry_adapter(max_retries))  # tied to http and https
     return session
 
 

src/zimscraperlib/filesystem.py

@@ -1,14 +1,12 @@
-#!/usr/bin/env python
-# vim: ai ts=4 sts=4 et sw=4 nu
-
 """ Files manipulation tools
 
     Shortcuts to retrieve mime type using magic"""
 
-from __future__ import annotations
-
 import os
 import pathlib
+from contextlib import contextmanager
+from tempfile import TemporaryDirectory
+from typing import Any
 
 import magic
 
@@ -46,3 +44,20 @@ def delete_callback(fpath: str | pathlib.Path):
     """helper deleting passed filepath"""
 
     os.unlink(fpath)
+
+
+@contextmanager
+def path_from(path: pathlib.Path | TemporaryDirectory[Any] | str):
+    """Context manager to get a Path from a path as string, Path or TemporaryDirectory
+
+    Since scraperlib wants to manipulate only Path, scrapers might often needs this
+    to create a path from what they have, especially since TemporaryDirectory context
+    manager returns a string which is not really handy.
+    """
+    if isinstance(path, pathlib.Path):
+        yield path
+    elif isinstance(path, TemporaryDirectory):
+        with path as pathname:
+            yield pathlib.Path(pathname)
+    else:
+        yield pathlib.Path(path)

src/zimscraperlib/fix_ogvjs_dist.py

@@ -1,11 +1,5 @@
-#!/usr/bin/env python3
-# vim: ai ts=4 sts=4 et sw=4 nu
-
-
 """ quick script to fix videojs-ogvjs so that it triggers on webm mimetype """
 
-from __future__ import annotations
-
 import logging
 import pathlib
 import sys

src/zimscraperlib/html.py

@@ -1,8 +1,4 @@
-#!/usr/bin/env python
-# vim: ai ts=4 sts=4 et sw=4 nu
-
 """ Tools to work with HTML contents """
-from __future__ import annotations
 
 import pathlib
 from typing import BinaryIO, TextIO
@@ -43,9 +39,7 @@ def find_language_in(content: str | BinaryIO | TextIO, mime_type: str) -> str:
         for key in keylist:
             node = soup.find(nodename)
             if node:
-                if not isinstance(node, element.Tag) or (
-                    isinstance(node, element.Tag) and not node.has_attr(key)
-                ):
+                if not isinstance(node, element.Tag) or not node.has_attr(key):
                     continue
                 if (
                     nodename == "meta"