frostming / pbs-installer

MIT License
9 stars 3 forks source link

Awkward type checking for `get_download_link` #6

Open ncoghlan opened 2 months ago

ncoghlan commented 2 months ago

Prompted by https://github.com/pdm-project/pdm/issues/3049, I recently switched to using pdm and pbs_installer via their Python APIs, rather than invoking pdm python install in a subprocess.

This change resulted in complaints from mypy:

error: Argument "implementation" to "get_download_link" has incompatible type "str"; expected "Literal['cpython', 'pypy']"

This type error is emitted from a code snippet replicated from pdm:

    implementation, _, version = request.rpartition("@")
    implementation = implementation.lower() or "cpython"
    version, _, arch = version.partition("-")
    arch = "x86" if arch == "32" else (arch or THIS_ARCH)

    ver, python_file = get_download_link(version, implementation=implementation, arch=arch, build_dir=False)

To get it to type check I had to change the call to the following:

    checked_impl : Literal["cpython", "pypy"] = "cpython"
    if implementation == "pypy":
        checked_impl = "pypy"
    elif implementation != "cpython":
        raise ValueError(f"Unknown interpreter implementation: {implementation}")
    ver, python_file = get_download_link(version, implementation=checked_impl, arch=arch, build_dir=False)

If https://github.com/python/mypy/issues/12535 were implemented, the check could potentially be simplified, but it's still awkward to have to replicate checking that pdm is going to be doing anyway due to the strict input signature on the public API.

ncoghlan commented 2 months ago

To allow callers to pass arbitrary strings and let the runtime error escape, PDM could declare an overload that accepts str: https://typing.readthedocs.io/en/latest/spec/literal.html#interactions-with-overloads

With that, the original code would typecheck.

Another option would be to include a suitable typeguard somewhere in the public pdm API:

from __future__ import annotations

if TYPE_CHECKING:
    from typing import TypeGuard, Literal
    PythonImplementation = Literal["cpython", "pypy"]

_known_python_implementations: frozenset[PythonImplementation] = frozenset("cpython", "pypy")

def is_python_implementation(val: str) -> TypeGuard[PythonImplementation]:
    """Determines whether the given string names a known Python implementation"""
    return val in _known_python_implementations

Then the strictly typed client side code would become:

    if not is_python_implementation(implementation):
       raise ValueError(f"Unknown interpreter implementation: {implementation}")
    ver, python_file = get_download_link(version, implementation=implementation, arch=arch, build_dir=False)