django / asgiref

ASGI specification and utilities
https://asgi.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
1.46k stars 207 forks source link

Add type annotations to asgiref functions #429

Open davitacols opened 8 months ago

davitacols commented 8 months ago

Changes Made

Why

Refactored the code to enhance readability and maintainability. The changes aim to improve understanding and make the codebase more consistent.

Checklist

Notes for Reviewers

Please review for clarity and consistency. No functional changes were introduced; the focus is on code organization and readability.

andrewgodwin commented 8 months ago

Looks like you've got some work to do to make the tests and the linter happy! I'd recommend running them locally; GitHub won't run them for you on every commit because you're a first-time contributor.

davitacols commented 8 months ago

all 3 tests passed locally

davitacols commented 8 months ago

Screenshot (66) tests result

andrewgodwin commented 8 months ago

Unfortunately the tests passing locally isn't enough - we have a very particular CI setup because asgiref is incredibly hard to test correctly. If you're not already running it, I might recommend running the tests via tox as it will ensure that you have all the right Python versions and fresh virtualenvs for each?

davitacols commented 8 months ago

Right, there is no problem. I will go ahead with tox

davitacols commented 8 months ago

mypy keeps failing. rest of the tests looks good. Screenshot (67)

andrewgodwin commented 8 months ago

Unfortunately mypy is exactly what you need to satisfy; there's a reason some of this typing hasn't been done - it's very complex to type correctly, as you can see!

davitacols commented 8 months ago

Unfortunately mypy is exactly what you need to satisfy; there's a reason some of this typing hasn't been done - it's very complex to type correctly, as you can see!

Will continue working on it till it passes the test.

davitacols commented 8 months ago
import inspect
from typing import Any, Awaitable, Callable

from .sync import iscoroutinefunction

def is_double_callable(application: Any) -> bool:
    """
    Tests to see if an application is a legacy-style (double-callable) application.
    """
    if getattr(application, "_asgi_single_callable", False):
        return False
    if getattr(application, "_asgi_double_callable", False):
        return True
    if inspect.isclass(application):
        return True
    if hasattr(application, "__call__"):
        if iscoroutinefunction(application.__call__):
            return False
    return not iscoroutinefunction(application)

async def double_to_single_callable(application: Any) -> Callable[..., Awaitable[None]]:
    """
    Transforms a double-callable ASGI application into a single-callable one.
    """

    async def new_application(
        scope: Any,
        receive: Callable[..., Awaitable[Any]],
        send: Callable[[Any], Awaitable[None]],
    ) -> None:

        instance = application(scope)
        await instance(receive, send)

    return new_application

def guarantee_single_callable(application: Any) -> Any:
    """
    Takes either a single- or double-callable application and always returns it
    in single-callable style. Use this to add backward compatibility for ASGI
    2.0 applications to your server/test harness/etc.
    """
    if is_double_callable(application):
        application = double_to_single_callable(application)
    return application

This tends to pass mypy test but tox finds some faults in the test_compatibility.py

tynor commented 7 months ago

I am also interested in getting type annotations added to asgiref.compatibility. :) I have a branch in my fork with working type annotations. I have not run it against the full tox matrix, but it does pass mypy type checking (on Python 3.11) without using Any anywhere (pun not intended).

I'm not experienced enough with Github to know how best to contribute to a PR already in progress (should I just open another one?) but the code is there if @davitacols is interested in integrating it into this PR.

davitacols commented 7 months ago

I am also interested in getting type annotations added to asgiref.compatibility. :) I have a branch in my fork with working type annotations. I have not run it against the full tox matrix, but it does pass mypy type checking (on Python 3.11) without using Any anywhere (pun not intended).

I'm not experienced enough with Github to know how best to contribute to a PR already in progress (should I just open another one?) but the code is there if @davitacols is interested in integrating it into this PR.

I am so glad to hear about such a progress. Moreover, you can post the compatibilty.py code here in the comment or you can make a fresh PR to my original PR.