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

Use ParamSpec to type check Actor args #515

Merged
merged 3 commits into from
Dec 28, 2022

Conversation

orsinium
Copy link
Contributor

@orsinium orsinium commented Dec 7, 2022

The PR introduces ParamSpec (PEP 612) to type annotate that Actor.__call__ and similar expect all the same arguments as the wrapped handler function. A few clarifications on the black magic that happening:

  1. I import ParamSpec from typing-extensions to be compatible with all Python versions.
  2. I have this import and ParamSpec definition inside of if TYPE_CHECKING block, so dramatiq doesn't depend on typing-extensions at runtime. At static type checking time by mypy, typing-extensions are guaranteed to be available because they are a dependency of mypy (and pyright has their stubs bundled).
  3. I added from __future__ import annotations, so that imports are lazy. See PEP 563. I've added it because initially I made it so that ParamSpec is not available at runtime. I later had to mock it for runtime (see the next point) but decided to leave this lazy annotations hack, it's quite handy on its own for many reasons.
  4. Generic is a base class, not an annotation, and so its arguments are evaluated at runtime. I tried to pass the ParamSpec as a string, but then it explodes inruntime with TypeError. So, I replace ParamSpec with TypeVar in runtime.
  5. ParamSpec can be used only with *args and **kwargs, so message_with_options and send_with_options cannot be annotated.

@Bogdanp Bogdanp merged commit 6ee6802 into Bogdanp:master Dec 28, 2022
@orsinium orsinium deleted the param-spec branch December 28, 2022 08:53
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

Successfully merging this pull request may close these issues.

2 participants