chore: add incremental type checking with mypy

[denismaciel]

The PR sets up mypy to be adopted incrementally in the code base.

The idea is: we make mypy ignore all files. We then annotate one file at a time. As we annotate files, we remove them from being ignored by mypy.

I have type-annotated the apy.config module while ignoring all other modules.

The workflow for annotating an extra file is:

1. Remove the file from the ignore list in pyproject.toml
2. Run mypy src to identify the violations
3. Fix the violations
4. Be happy and type-safe

Do you think this works for you, @lervag?

If you're happy with the approach, I will annotate/fix other files and add a type-check step to the Github Action.

Looking forward to your feedback.

Greetings from Hamburg!

[lervag]

The PR sets up mypy to be adopted incrementally in the code base.

Great, thanks!

The idea is: we make mypy ignore all files. We then annotate one file at a time. As we annotate files, we remove them from being ignored by mypy.

Good idea.

The workflow for annotating an extra file is: …

This sounds reasonable as a workflow. I guess it shouldn't take that long either, it is not really so much code.

Do you think this works for you, @lervag?

Yes!

If you're happy with the approach, I will annotate/fix other files and add a type-check step to the Github Action.

Good; I guess adding the type-check step to the GHA could be useful as part of this PR?

Greetings from Hamburg!

Greetings from Norway :)

[denismaciel]

@lervag I have addressed your comments and added mypy to CI.

I was also thinking we could make the cfg dictionary a dataclasses.dataclass eventually.

[lervag]

I was also thinking we could make the cfg dictionary a dataclasses.dataclass eventually.

Seems like a good idea!

I think this is very good! The final thing I'm thinking about is if we should include the tests here?

[ckp95]

I was actually planning to make a PR with a refactor of the config to use a dataclass. It's cleaner because you see the fields and the types all in one place. type checker can work with it better than a heterogeneously typed dict.

[ckp95]

the idea is something like this

import json
import os
from dataclasses import dataclass, field
from pathlib import Path
from typing import Optional, Any

from platformdirs import user_config_path, user_data_path


def env_path(name: str, must_exist_if_specified=True) -> Optional[Path]:
    if (raw_path := os.environ.get(name)) is None:
        return None

    if must_exist_if_specified and not (path := Path(raw_path)).exists():
        raise FileNotFoundError(f"No such path: {name}={path}")

    return path


def get_raw_config() -> dict[str, Any]:
    cfg_dir = env_path("APY_CONFIG") or user_config_path("apy")
    if not (cfg_path := cfg_dir / "apy.json").exists():
        return {}

    return json.loads(cfg_path.read_text())


def get_base_path() -> Path:
    return env_path("APY_BASE") or env_path("ANKI_BASE") or user_data_path("Anki2")


@dataclass
class Config:
    base_path: Path = field(default_factory=get_base_path)
    img_viewers: dict[str, list[str]] = field(
        default_factory=lambda: {"svg": ["display", "-density", "300"]}
    )
    img_viewers_default: list[str] = field(default_factory=lambda: ["feh"])
    markdown_models: list[str] = field(default_factory=lambda: ["Basic"])
    presets: dict[str, dict] = field(default_factory=dict)
    profile: Optional[str] = None
    query: str = "tag:marked OR -flag:0"

    def __post_init__(self) -> None:
        if not self.base_path.exists():
            raise FileNotFoundError(f"Could not find Anki base path: {self.base_path}")

        self.base_path = self.base_path.absolute()
        self.presets.setdefault("default", {"model": "Basic", "tags": []})


cfg = Config(**get_raw_config())

haven't tested this or modified the other modules to account for the changes yet.

[ckp95]

Would it make more sense for me to push this dataclass config stuff to this PR, or to make a separate one?

[lervag]

Could you make a new PR? I'll merge this one probably later today. If you make the PR based on this branch and later rebase it on master after this is merged, then it should be quite clean.

[lervag]

I've merged this now; thanks!