Activate strict mode and fix type hints

[benoit74]

Fix #212

Changes:

• activate pyright strict mode (obviously)
• add all missing type hints and more pyright: ignore[...] hints (always qualified)
• remove all headers which are not maintained (shebang + vim config)
• remove all from __future__ import annotations since we target only Python 3.12, those are useless now
• breaking: remove with_process argument of zimscraperlib.video.encoding.reencode and always return the process ; those who don't mind about this second argument can just drop it, this simplify the codebase and the signature and the type hints
• expose zimscraperlib.download.get_retry_adapter since it is used in zimscraperlib.zim.provider
• properly expose __all__ in zimscraperlib/video/__init__.py (import where there but not used hence mostly useless)
• fix type hint of mappings attribute of zimscraperlib.video.config.Config: mappings values are always a string since they are in fact keys)
• simply type hint of options and defaults attributes of zimscraperlib.video.config.Config: options and defaults values are always a string or None since they are intended to be passed to the CLI as argument ; authorizing int and bool is not properly handled in the code, not needed, not used.
• I had to rewrite a bit the metadata stuff to please the type checker:
  • add a generic and abstract MetadataBase class which holds most property and can accomodate any value type, but obviously is abstract because it has no idea how to transform any value into a libzim value (this is the purpose of real implementation)
  • add a convenient AnyMetadata type alias
  • use a bounded TypeVar U to properly type all class decorators so that they properly accommodate to the class they are used on while still knowing it has access to all attributes of the MetadataBase class (it took me ages to figure this out, so worth mentioning and have in mind)
  • I had to modify (again) get_reserved_names method which broke for some reason ; I feel like new implementation is indeed simpler (it does not need to rely on the weird globals())
  • I decided to modify _log_metadata and drop the is None test ; type checker was complaining that this is not possible to happen (which is True if scraper use type hints properly and/or use only public methods which are protected by beartype ... only case where it can happen is if you do not use type checker and you manipulate directly the _metadata attribute ...) and message logged is not awful in such a situation ... is improper metadata type: NoneType: None instead of ... is None

Nota:

• there is an important typing issue in zimscraperlib.zim.creator.Creator with methods add_metadata and add_item. I had to set the pragma # pyright: ignore[reportIncompatibleMethodOverride] because these methods override the base class methods (from pylibzim) with incompatible signature. This is not recommended. I feel like it is still acceptable since very convenient, and not a big deal (we never use the base class directly in scrapers in fact, and we do not have multiple child implementation of the base class with different signatures, so this is not really confusing).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b7b5e46) to head (78fa538).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #220   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           39        39           
  Lines         2417      2455   +38     
  Branches       322       331    +9     
=========================================
+ Hits          2417      2455   +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rgaudin]

Thank you! That must have been long and painful.

I really like the MetadataBase and the get_reserved_names ; very nice!
I dont mind the add_metata(), it was done on purpose.

My only concern regards accepting str for paths. We should not go this way IMO.

A way to resolve it might be to create a custom context manager like

from pathlib import Path
from tempfile import TemporaryDirectory

@contextmanager
def path_from(path: Path | TemporaryDirectory | str):
    if isinstance(path, Path):
        yield path
    elif callable(hasattr(path, "__enter__")):
        with path as pathname:
            yield Path(pathname)
    else:
        yield Path(pathname)

You would use it like:

with path_from(existing_tmp_path or tempfile.TemporaryDirectory()) as tmp_dir:
    ...