-
Notifications
You must be signed in to change notification settings - Fork 304
Fix: make python 3.12 type X = ... syntax work with dependency injection
#2951
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
2c9703d
9eb262f
7e50413
80de12a
7846971
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| from contextlib import asynccontextmanager, contextmanager | ||
| from typing import Annotated, AsyncGenerator, Generator | ||
| from typing import Annotated, AsyncGenerator, Generator, TypeAlias | ||
|
|
||
| from pydantic import BaseModel | ||
|
|
||
|
|
@@ -88,12 +88,15 @@ class Output(BaseModel): | |
| chained_async_dep: str | ||
|
|
||
|
|
||
| type AsyncDep = Annotated[str, Depends(async_dep)] | ||
|
|
||
|
|
||
| # > Inject dependencies | ||
| @hatchet.task() | ||
| async def async_task_with_dependencies( | ||
| _i: EmptyModel, | ||
| ctx: Context, | ||
| async_dep: Annotated[str, Depends(async_dep)], | ||
| async_dep: AsyncDep, | ||
|
||
| sync_dep: Annotated[str, Depends(sync_dep)], | ||
| async_cm_dep: Annotated[str, Depends(async_cm_dep)], | ||
| sync_cm_dep: Annotated[str, Depends(sync_cm_dep)], | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |||||||||||
| get_origin, | ||||||||||||
| get_type_hints, | ||||||||||||
| ) | ||||||||||||
| from typing_extensions import TypeAliasType | ||||||||||||
|
||||||||||||
| from typing_extensions import TypeAliasType | |
| try: | |
| from typing import TypeAliasType # type: ignore[attr-defined] | |
| except ImportError: # pragma: no cover - fallback for older Python versions | |
| from typing_extensions import TypeAliasType |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The runtime class for type X = ... in Python 3.12 is typing.TypeAliasType. Depending on how typing_extensions.TypeAliasType is implemented in the installed version, isinstance(annotation, TypeAliasType) can be brittle across environments (e.g., if it’s not the same class object). Prefer importing TypeAliasType from typing when available with a fallback to typing_extensions, or use a small compatibility check that handles both stdlib and typing_extensions implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong, type_extensions just imports from typing if it's available.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -148,7 +148,7 @@ class StepType(str, Enum): | |
| def is_async_fn( | ||
| fn: TaskFunc[TWorkflowInput, R], | ||
| ) -> TypeGuard[AsyncFunc[TWorkflowInput, R]]: | ||
| return asyncio.iscoroutinefunction(fn) | ||
| return inspect.iscoroutinefunction(fn) | ||
|
||
|
|
||
|
|
||
| def is_sync_fn( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using PEP 695
type AsyncDep = ...syntax makes this example file a SyntaxError on Python < 3.12. If the SDK/examples are intended to run on older supported Python versions, switch to a backwards-compatible alias (e.g., assignment-based alias, optionally withTypeAlias) instead of thetypestatement, or gate the example so it’s only executed on 3.12+.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@StefanBRas it'd be worth testing this on 3.10 and 3.11 to confirm, because we'll need to handle this case if this is correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can handle TypeAlias on any version as well, but I wasn't sure how you want to organize the tests since the
type X = ...is a syntax error on <3.12 so it is not even possible to import a module with the syntax.I'm thinking to just add a module with the type that is then conditionally imported depending on the version.
Do you want a test that is just skipped with pytest skipped if version is <3.12 or just that the test only uses type syntax if >=3.12?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have this in
utils/typing.py- something similar might help here too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately that's not enough:
The syntax can't be in the file at all, not even in a class scope.
Maybe it's possible to construct the
TypeAliasTypeobject itself without the syntax, but that feels more brittle.My idea is to have something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrkaye97 I put in a test now that does that split.
I saw that the example file was used in the docs too, so I put it such that it didn't interfere with that.
That also means that it probably could be refactored a bit to be nicer, but I think it's okay as is