Typing for sync/async decorators #298

shughes-uk · 2021-09-25T19:34:37Z

This is a first pass draft, fairly sure it's broken for python versions lower than 3.8.

Seems to work but i'm not sure how to test it right this moment, i'm used to using pyright and will have to investigate how to use mypy

One thing that confused me a bit is that the async_to_sync function accepts awaitable but it actually seems to be looking for a callable that returns an awaitable? I could use some clarification there.

I'm also not sure what naming convention to use for the TypeVar/Paramspec pairs.

Looks like I also introduce some formatting errors to correct!

shughes-uk · 2021-09-26T04:18:00Z

Seems I have some work to do with mypy and this!

andrewgodwin · 2021-09-26T16:05:53Z

No worries - take the time you need to get everything working! I might suggest you mark the PR as a draft, though, so that way you can move it to "ready for review" when you think you've got it all ready to go.

shughes-uk · 2021-09-26T19:24:31Z

mypy seems not to support paramspec yet so this might take a little while unless you want to switch to pyright or pyre python/mypy#8645

andrewgodwin · 2021-10-04T01:11:50Z

I'm not really feeling like switching typechecker right at this moment, unfortunately. Sorry for delaying this yet further!

graingert · 2021-10-15T08:11:23Z

This is a first pass draft, fairly sure it's broken for python versions lower than 3.8.

typing_extensions.ParamSpec has been backported to python 3.6 so it should still work fine.

Seems to work but i'm not sure how to test it right this moment, i'm used to using pyright and will have to investigate how to use mypy

mypy on pypi currently doesn't support ParamSpec, but now that python/mypy#10883 and python/mypy#10866 and python/mypy#10862 have been merged the version on m*ster at least treats ParamSpec as Any so as soon as mypy releases this change will be no worse for mypy users.

One thing that confused me a bit is that the async_to_sync function accepts awaitable but it actually seems to be looking for a callable that returns an awaitable? I could use some clarification there.

That should definitely be a Callable[P, Awaitable[R]]

I'm also not sure what naming convention to use for the TypeVar/Paramspec pairs.

you can just use:

P = ParamSpec("P")
R = TypeVar("R")

you don't need define new ParamSpec placeholder variables for every use - you can use the same one over and over.

Looks like I also introduce some formatting errors to correct!

graingert · 2021-10-15T08:17:53Z

asgiref/sync.py

+        loop,
+        source_task,
+        exc_info,


probably worth defining an _ExcInfo type alias here:

_ExcInfo = Tuple[Optional[Type[BaseException], None], Optional[BaseException], Optional[types.TracebackType]]

Suggested change

loop,

source_task,

exc_info,

loop: asyncio.AbstractEventLoop,

source_task: asyncio.Task[object],

exc_info: _ExcInfo,

graingert · 2021-10-15T08:18:21Z

asgiref/sync.py

+        self,
+        args,
+        kwargs,
+        call_result: a_cls_return,


Suggested change

call_result: a_cls_return,

call_result: Future[a_cls_return],

graingert · 2021-10-15T08:24:34Z

asgiref/sync.py

+        args,
+        kwargs,


you need to type annotate all the parameters for mypy to check this function

It's probably worth making all the parameters positional only using a closure so you can use *args: P.args and **kwargs: P.kwargs:

def _main_wrap(self, call_result: Future[R], source_thread: threading.Thread, exc_info: ExcInfo, context: Optional[Context]) -> Callable[P, Coroutine[Any, Any, None]: async def wrap(*args: P.args, **kwargs: P.kwargs) -> None: ... return wrap

you'd then need to call it like this

awaitable = self._main_wrap( call_result, source_thread, sys.exc_info(), context )(*args, **kwargs)

shughes-uk · 2021-10-19T00:31:02Z

Made a little more progress.

Switching to using the main mypy branch for now (to be resolved before merging).
Mypy seems to not like combining paramspec with generics
Future is only generic in 3.9+ and will throw exceptions in previous versions, removed it for now

shughes-uk · 2021-10-19T06:21:08Z

Not entirely sure whats going on but the checks appears to have been running for many hours 😱

graingert · 2021-10-19T06:35:30Z

Future is only generic in 3.9+ and will throw exceptions in previous versions, removed it for now

You'll need to use a forward reference with "Future[R]"

graingert · 2021-10-19T18:48:40Z

asgiref/sync.py

@@ -319,6 +318,10 @@ async def main_wrap(
                _restore_context(context[0])

            current_task = SyncToAsync.get_current_task()
+            if not current_task:


Probably best to use an assertion instead eg:

assert current_task is not None

This way it's not considered an un covered branch, and is idiomatic mypy type assertion

ah perfect thank you

shughes-uk · 2021-10-19T18:56:20Z

3.9 Tests are passing and these are the mypy issues left. Most of which seem to be caused by mypy not recognizing Generic's can accept ParamSpecs.

asgiref/sync.py:126: error: Free type variable expected in Generic[...]
asgiref/sync.py:159: error: "Callable[..., Awaitable[R]]" has no attribute "__self__"
asgiref/sync.py:183: error: Name "P.args" is not defined
asgiref/sync.py:183: error: Name "P.kwargs" is not defined
asgiref/sync.py:310: error: Name "P.args" is not defined
asgiref/sync.py:311: error: Name "P.kwargs" is not defined
asgiref/sync.py:348: error: Free type variable expected in Generic[...]
asgiref/sync.py:432: error: Name "P.args" is not defined
asgiref/sync.py:432: error: Name "P.kwargs" is not defined
asgiref/sync.py:441: error: Argument 1 to "get" of "ContextVar" has incompatible type "None"; expected "Union[ContextVar[str], str]"
asgiref/sync.py:498: error: Returning Any from function declared to return "R"
asgiref/sync.py:506: error: Function is missing a return type annotation
asgiref/sync.py:512: error: Name "P.args" is not defined
asgiref/sync.py:513: error: Name "P.kwargs" is not defined
asgiref/sync.py:569: error: "SyncToAsync" expects 1 type argument, but 2 given
asgiref/sync.py:569: error: Invalid location for ParamSpec "P"
asgiref/sync.py:569: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
asgiref/sync.py:578: error: "SyncToAsync" expects 1 type argument, but 2 given
asgiref/sync.py:578: error: Invalid location for ParamSpec "P"
asgiref/sync.py:578: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'

graingert · 2021-10-19T19:00:59Z

3.9 Tests are passing and these are the mypy issues left. Most of which seem to be caused by mypy not recognizing Generic's can accept ParamSpecs.

asgiref/sync.py:126: error: Free type variable expected in Generic[...]
asgiref/sync.py:159: error: "Callable[..., Awaitable[R]]" has no attribute "__self__"
asgiref/sync.py:183: error: Name "P.args" is not defined
asgiref/sync.py:183: error: Name "P.kwargs" is not defined
asgiref/sync.py:310: error: Name "P.args" is not defined
asgiref/sync.py:311: error: Name "P.kwargs" is not defined
asgiref/sync.py:348: error: Free type variable expected in Generic[...]
asgiref/sync.py:432: error: Name "P.args" is not defined
asgiref/sync.py:432: error: Name "P.kwargs" is not defined
asgiref/sync.py:441: error: Argument 1 to "get" of "ContextVar" has incompatible type "None"; expected "Union[ContextVar[str], str]"
asgiref/sync.py:498: error: Returning Any from function declared to return "R"
asgiref/sync.py:506: error: Function is missing a return type annotation
asgiref/sync.py:512: error: Name "P.args" is not defined
asgiref/sync.py:513: error: Name "P.kwargs" is not defined
asgiref/sync.py:569: error: "SyncToAsync" expects 1 type argument, but 2 given
asgiref/sync.py:569: error: Invalid location for ParamSpec "P"
asgiref/sync.py:569: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'
asgiref/sync.py:578: error: "SyncToAsync" expects 1 type argument, but 2 given
asgiref/sync.py:578: error: Invalid location for ParamSpec "P"
asgiref/sync.py:578: note: You can use ParamSpec as the first argument to Callable, e.g., 'Callable[P, int]'

Is there a ticket for this in mypy? I think it's supposed to be ignored

shughes-uk · 2021-10-19T19:09:47Z

Is there a ticket for this in mypy? I think it's supposed to be ignored

python/mypy#11362 created one with minimal reproduction code

shughes-uk · 2021-10-19T19:21:26Z

3.6 tests are failing with a circular import caused by the context shenanigans on Local. Looks kinda tricky to solve

shughes-uk · 2021-11-25T22:47:20Z

@andrewgodwin @graingert I've updated this with a workaround based on the current (unreleased) mypy version. Temporarily installing mypy from the git version. Once mypy cuts a release this should be good to go. No generics required, but maybe a little hacky.

Other option is to wait for generic support in mypy and a release. Which who knows maybe that'll happen before the next release rendering the workaround redundant.

andrewgodwin · 2021-11-29T21:15:00Z

Will current versions of mypy-in-the-wild choke on the new stuff, btw, or will they ignore it and not typecheck it? Considering the type annotations here are mostly for the benefit of users rather than asgiref itself, I want to ensure it's not going to torpedo current users.

graingert · 2021-11-29T21:17:25Z

Sadly mypy 0.910 will choke on this. I think the work-around is to provide these aliases in a new module eg asgiref.sync_paramspec

shughes-uk · 2021-11-30T02:33:23Z

Yes this will blow up on wild mypy. Once they roll out a new release this could be merged without too much worry unless you intend for backwards compatibility with older mypy versions.

andrewgodwin · 2021-12-04T18:22:19Z

Ugh, yeah, this is tricky because end-users mypy installations is the thing we care about at the end of the day. No way to somehow erase ParamSpec and fall back to Any or something on those older versions, I'm guessing?

setup.cfg

Co-authored-by: Thomas Grainger <[email protected]>

graingert · 2021-12-22T18:02:39Z

mypy 0.930 is out with experimental support for ParamSpec! https://mypy-lang.blogspot.com/2021/12/mypy-0930-released.html?m=1

This is particularly good as it has support for Generic[P, R]

lexicalunit · 2022-01-25T13:09:52Z

Given that Python 3.6 is no longer supported as of asgiref version 3.5.0, is now a good time to revisit this discussion? Sorry, I've just been following along here and am really excited to see this feature land at some point 😄

andrewgodwin · 2022-01-29T19:31:26Z

Maybe? I don't keep tight track of typing stuff right now so I'm not quite sure where things sit in terms of this working with 3.7.

graingert · 2022-01-29T19:48:57Z

All versions of python support ParamSpec, even Python 2.7 all you need to do is import it in an "if TYPE_CHECKING:" block, or use the typing_extensions backport

shughes-uk marked this pull request as draft September 26, 2021 19:20

graingert reviewed Oct 15, 2021

View reviewed changes

shughes-uk force-pushed the resolve-270 branch from ab6c0e3 to f5a5706 Compare October 18, 2021 17:30

graingert reviewed Oct 19, 2021

View reviewed changes

typing for sync/async decorators

b941db3

shughes-uk force-pushed the resolve-270 branch 5 times, most recently from f729c0b to e555890 Compare November 25, 2021 22:42

workaround

c9b210a

shughes-uk force-pushed the resolve-270 branch from e555890 to c9b210a Compare November 25, 2021 22:43

graingert reviewed Dec 16, 2021

View reviewed changes

setup.cfg Outdated Show resolved Hide resolved

Update setup.cfg

ff26ff4

Co-authored-by: Thomas Grainger <[email protected]>

This was referenced Feb 19, 2023

Add ParamSpec typing for AsyncToSync and SyncToAsync #373

Closed

Preserving typing on async_to_sync/sync_to_async wrapped functions #270

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typing for sync/async decorators #298

Typing for sync/async decorators #298

shughes-uk commented Sep 25, 2021

shughes-uk commented Sep 26, 2021

andrewgodwin commented Sep 26, 2021

shughes-uk commented Sep 26, 2021

andrewgodwin commented Oct 4, 2021

graingert commented Oct 15, 2021 •

edited

Loading

graingert Oct 15, 2021

graingert Oct 15, 2021

graingert Oct 15, 2021 •

edited

Loading

shughes-uk commented Oct 19, 2021

shughes-uk commented Oct 19, 2021

graingert commented Oct 19, 2021

graingert Oct 19, 2021

graingert Oct 19, 2021

shughes-uk Oct 19, 2021

shughes-uk commented Oct 19, 2021 •

edited

Loading

graingert commented Oct 19, 2021

shughes-uk commented Oct 19, 2021 •

edited

Loading

shughes-uk commented Oct 19, 2021

shughes-uk commented Nov 25, 2021 •

edited

Loading

andrewgodwin commented Nov 29, 2021

graingert commented Nov 29, 2021

shughes-uk commented Nov 30, 2021

andrewgodwin commented Dec 4, 2021

graingert commented Dec 22, 2021 •

edited

Loading

lexicalunit commented Jan 25, 2022

andrewgodwin commented Jan 29, 2022

graingert commented Jan 29, 2022

-        loop,
-        source_task,
-        exc_info,
+        loop: asyncio.AbstractEventLoop,
+        source_task: asyncio.Task[object],
+        exc_info: _ExcInfo,

	call_result: a_cls_return,
	call_result: Future[a_cls_return],

Typing for sync/async decorators #298

Are you sure you want to change the base?

Typing for sync/async decorators #298

Conversation

shughes-uk commented Sep 25, 2021

shughes-uk commented Sep 26, 2021

andrewgodwin commented Sep 26, 2021

shughes-uk commented Sep 26, 2021

andrewgodwin commented Oct 4, 2021

graingert commented Oct 15, 2021 • edited Loading

graingert Oct 15, 2021

Choose a reason for hiding this comment

graingert Oct 15, 2021

Choose a reason for hiding this comment

graingert Oct 15, 2021 • edited Loading

Choose a reason for hiding this comment

shughes-uk commented Oct 19, 2021

shughes-uk commented Oct 19, 2021

graingert commented Oct 19, 2021

graingert Oct 19, 2021

Choose a reason for hiding this comment

graingert Oct 19, 2021

Choose a reason for hiding this comment

shughes-uk Oct 19, 2021

Choose a reason for hiding this comment

shughes-uk commented Oct 19, 2021 • edited Loading

graingert commented Oct 19, 2021

shughes-uk commented Oct 19, 2021 • edited Loading

shughes-uk commented Oct 19, 2021

shughes-uk commented Nov 25, 2021 • edited Loading

andrewgodwin commented Nov 29, 2021

graingert commented Nov 29, 2021

shughes-uk commented Nov 30, 2021

andrewgodwin commented Dec 4, 2021

graingert commented Dec 22, 2021 • edited Loading

lexicalunit commented Jan 25, 2022

andrewgodwin commented Jan 29, 2022

graingert commented Jan 29, 2022

graingert commented Oct 15, 2021 •

edited

Loading

graingert Oct 15, 2021 •

edited

Loading

shughes-uk commented Oct 19, 2021 •

edited

Loading

shughes-uk commented Oct 19, 2021 •

edited

Loading

shughes-uk commented Nov 25, 2021 •

edited

Loading

graingert commented Dec 22, 2021 •

edited

Loading