Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Awkward type checking for get_download_link #6

Open
ncoghlan opened this issue Jul 30, 2024 · 1 comment
Open

Awkward type checking for get_download_link #6

ncoghlan opened this issue Jul 30, 2024 · 1 comment

Comments

@ncoghlan
Copy link

Prompted by pdm-project/pdm#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 python/mypy#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
Copy link
Author

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant