-
Notifications
You must be signed in to change notification settings - Fork 9
feat(spider-py): Add support for client-end task graph grouping and chaining. #191
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
Changes from 201 commits
d2ac198
cc8a097
7f68bdc
2394649
6ff3e33
7ae8a78
56502f9
6f6becf
a74523d
2345d22
77ee494
38b86c8
3698339
369e9f1
90aa5a2
1769c95
edaa834
7faac8f
f494a90
eb01bb2
ff2fe1c
d476e42
850126d
65841a0
8564fc2
a6e7d29
573b448
6ad72c3
ba7c6e5
209acb1
406b514
66892f5
80f0a10
3186a42
0925938
6936a9b
7407090
6a95b24
f29dcba
453f209
d1011c7
a7dc642
a7d92c2
1c6213b
0f13d07
14d6326
55a727a
c98fea1
ef64454
09ba8ad
4e7237e
1f944d0
68992fc
1127b22
4c51c91
56878b2
bb428c9
944bd5e
d75bc15
4b7eca2
8ddbda7
0d998ec
6004d64
bcab5db
a50d25a
fbebb96
0322bd5
2a08155
dd29c23
570cf2f
9b2a175
32afe0a
0b62f73
7384158
70ef90a
1aae0cb
2f9b4a0
4e080ac
b09515c
bad9675
e009ff4
f11a3b6
a0407a7
aa9a8de
3c7e794
ca4ec29
b4d6576
23f31cd
0167ac0
5a9cb76
f447614
a84de83
f94460f
2d1b6fc
5a35122
061d101
451a80e
de149ee
d332dbe
80b6772
f2ebaa6
8c27b16
1b7ab77
b7f1884
a7aa6b0
95b98c8
e7b119c
8ffbd03
5765acf
427dd6b
a9c9e57
213d2fc
a1a86f3
97af71b
74f31e2
7d95706
96b5324
14ea6fb
854cd11
5565c9e
a9791e5
2504429
7ea4928
01dd0c6
c6ccce6
4cec09b
e022404
8894b90
01c9c64
7753153
dd59915
b86d47c
ac1d4ee
0d38c43
438f6e7
2caad5d
c6815ce
61e0651
8b13abd
50b2b25
519dcd8
8e2786b
a8c8641
9fabc44
e558f67
09d3fd0
ae2a935
008c6fb
cd60dc6
720265f
ebe64c6
404cf3b
3f9fe04
4d10f88
fe4121f
f3ac001
95b7773
a72e4a6
f289e5a
2e829bc
ec6802d
3685dd7
2862f01
03af08d
c3dda18
2a1f33e
0e4452c
3705ef3
fb8938a
9ed09be
ef827a3
220a2b0
56f7866
638d577
2bcc4a4
aa1723e
2469008
082476d
72239f7
e13acec
84a6256
fe904b0
75793de
72a0920
fecdc68
08ce667
4dc9623
ff76756
b9607ae
75c8b76
8e98c1e
fe306bc
d0fb7e2
6fe936d
82dab16
5ef3520
b879134
831e399
ff7f06a
a09b499
1051b51
bca62dd
437b39d
2c28e9d
0e87a54
6af6770
608036e
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 +1,11 @@ | ||
| """Spider python client.""" | ||
|
|
||
| from .task import TaskContext | ||
| from .task_graph import chain, group, TaskGraph | ||
|
|
||
| __all__ = [ | ||
| "TaskContext", | ||
| "TaskGraph", | ||
| "chain", | ||
| "group", | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| """Spider client Data module.""" | ||
|
|
||
|
|
||
| class Data: | ||
| """Represents a spider client data.""" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,101 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| """Spider client task module.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import inspect | ||||||||||||||||||||||||||||||||||||||||||||||
| from collections.abc import Callable | ||||||||||||||||||||||||||||||||||||||||||||||
| from types import FunctionType, GenericAlias | ||||||||||||||||||||||||||||||||||||||||||||||
| from typing import get_args, get_origin | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from spider_py import core | ||||||||||||||||||||||||||||||||||||||||||||||
| from spider_py.client.data import Data | ||||||||||||||||||||||||||||||||||||||||||||||
| from spider_py.core import TaskInput, TaskOutput, TaskOutputData, TaskOutputValue | ||||||||||||||||||||||||||||||||||||||||||||||
| from spider_py.type import to_tdl_type_str | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class TaskContext: | ||||||||||||||||||||||||||||||||||||||||||||||
| """Spider task context.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # TODO: Implement task context for use in task executor | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Check the TaskFunction signature at runtime. | ||||||||||||||||||||||||||||||||||||||||||||||
| # Enforcing static check for first argument requires the use of Protocol. However, functions, which | ||||||||||||||||||||||||||||||||||||||||||||||
| # are Callable, are not considered a Protocol without explicit cast. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| TaskFunction = Callable[..., object] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def _is_tuple(t: type | GenericAlias) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| :param t: | ||||||||||||||||||||||||||||||||||||||||||||||
| :return: Whether t is a tuple. | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| return get_origin(t) is tuple | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def _process_parameters(task: core.Task, signature: inspect.Signature) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| Checks the parameters validity and add them to the task. | ||||||||||||||||||||||||||||||||||||||||||||||
| :param task: | ||||||||||||||||||||||||||||||||||||||||||||||
| :param signature: | ||||||||||||||||||||||||||||||||||||||||||||||
| :raises TypeError: If the parameters are invalid. | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| params = list(signature.parameters.values()) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not params or params[0].annotation is not TaskContext: | ||||||||||||||||||||||||||||||||||||||||||||||
| msg = "First argument is not a TaskContext." | ||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||
| for param in params[1:]: | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Accept TaskContext subclasses for the first parameter. Using “is” restricts to the exact TaskContext class. Allowing subclasses makes the API more extensible without weakening checks. - if not params or params[0].annotation is not TaskContext:
+ first = params[0].annotation if params else None
+ if not params or not (first is TaskContext or (isinstance(first, type) and issubclass(first, TaskContext))):
msg = "First argument is not a TaskContext."
raise TypeError(msg)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| if param.kind in {inspect.Parameter.VAR_POSITIONAL, inspect.Parameter.VAR_KEYWORD}: | ||||||||||||||||||||||||||||||||||||||||||||||
| msg = "Variadic parameters are not supported." | ||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||
| if param.annotation is inspect.Parameter.empty: | ||||||||||||||||||||||||||||||||||||||||||||||
| msg = "Argument must have type annotation" | ||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||
| tdl_type_str = to_tdl_type_str(param.annotation) | ||||||||||||||||||||||||||||||||||||||||||||||
| task.task_inputs.append(TaskInput(tdl_type_str, None)) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def _process_return(task: core.Task, signature: inspect.Signature) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| Checks the return type validity and add them to the task. | ||||||||||||||||||||||||||||||||||||||||||||||
| :param task: | ||||||||||||||||||||||||||||||||||||||||||||||
| :param signature: | ||||||||||||||||||||||||||||||||||||||||||||||
| :raises TypeError: If the return type is invalid. | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| returns = signature.return_annotation | ||||||||||||||||||||||||||||||||||||||||||||||
| if returns is inspect.Parameter.empty: | ||||||||||||||||||||||||||||||||||||||||||||||
| msg = "Return type must have type annotation" | ||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||
| if _is_tuple(returns): | ||||||||||||||||||||||||||||||||||||||||||||||
| args = get_args(returns) | ||||||||||||||||||||||||||||||||||||||||||||||
| if Ellipsis in args: | ||||||||||||||||||||||||||||||||||||||||||||||
| msg = "Variable-length tuple return types are not supported." | ||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(msg) | ||||||||||||||||||||||||||||||||||||||||||||||
| for r in args: | ||||||||||||||||||||||||||||||||||||||||||||||
| tdl_type_str = to_tdl_type_str(r) | ||||||||||||||||||||||||||||||||||||||||||||||
| if r is Data: | ||||||||||||||||||||||||||||||||||||||||||||||
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputData())) | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputValue())) | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| tdl_type_str = to_tdl_type_str(returns) | ||||||||||||||||||||||||||||||||||||||||||||||
| if returns is Data: | ||||||||||||||||||||||||||||||||||||||||||||||
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputData())) | ||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputValue())) | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| tdl_type_str = to_tdl_type_str(r) | |
| if r is Data: | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputData())) | |
| else: | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputValue())) | |
| else: | |
| tdl_type_str = to_tdl_type_str(returns) | |
| if returns is Data: | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputData())) | |
| else: | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputValue())) | |
| tdl_type_str = to_tdl_type_str(r) | |
| if r is Data or (isinstance(r, type) and issubclass(r, Data)): | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputData())) | |
| else: | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputValue())) | |
| else: | |
| tdl_type_str = to_tdl_type_str(returns) | |
| if returns is Data or (isinstance(returns, type) and issubclass(returns, Data)): | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputData())) | |
| else: | |
| task.task_outputs.append(TaskOutput(tdl_type_str, TaskOutputValue())) |
🤖 Prompt for AI Agents
In python/spider-py/src/spider_py/client/task.py around lines 73-83, the current
checks use identity (is) to detect Data which fails for subclasses; change these
to use issubclass(...) where appropriate: for single return types call
to_tdl_type_str(returns) then if issubclass(returns, Data) append
TaskOutputData() else TaskOutputValue(); for tuple returns iterate the tuple
element types, compute each element's tdl_type_str and use issubclass(element,
Data) to decide between TaskOutputData() and TaskOutputValue(); keep using
to_tdl_type_str for type-string conversion and ensure the code handles
typing.Annotated/forward refs if already addressed elsewhere.
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """Spider client TaskGraph module.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| from spider_py import core | ||
| from spider_py.client.task import _create_task, TaskFunction | ||
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Sequence | ||
|
|
||
|
|
||
| class TaskGraph: | ||
| """ | ||
| Spider client TaskGraph class. | ||
| Wraps around the core TaskGraph class. | ||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| """ | ||
|
|
||
| def __init__(self) -> None: | ||
| """Initializes TaskGraph.""" | ||
| self._impl = core.TaskGraph() | ||
|
|
||
|
|
||
| def group(tasks: Sequence[TaskFunction | TaskGraph]) -> TaskGraph: | ||
| """ | ||
| Groups task functions and task graph into a single task graph. | ||
| :param tasks: List of task functions or task graphs. | ||
| :return: The new task graph. | ||
| """ | ||
| graph = TaskGraph() | ||
| for task in tasks: | ||
| if callable(task): | ||
| graph._impl.add_task(_create_task(task)) | ||
| else: | ||
| graph._impl.merge_graph(task._impl) | ||
|
|
||
| return graph | ||
sitaowang1998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| def chain(parent: TaskFunction | TaskGraph, child: TaskFunction | TaskGraph) -> TaskGraph: | ||
| """ | ||
| Chains two task functions or task graphs into a single task graph. | ||
| :param parent: | ||
| :param child: | ||
| :return: | ||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| :raises TypeError: If the parent outputs and child inputs do not match. | ||
| """ | ||
| if callable(parent): | ||
| task = _create_task(parent) | ||
| parent = TaskGraph() | ||
| parent._impl.add_task(task) | ||
| if callable(child): | ||
| task = _create_task(child) | ||
| child = TaskGraph() | ||
| child._impl.add_task(task) | ||
| graph = TaskGraph() | ||
| graph._impl = core.TaskGraph.chain_graph(parent._impl, child._impl) | ||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return graph | ||
sitaowang1998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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.
I feel like we should move these utility functions into core's task so that the creation of a task is more like RAII. The parameter and return type checks can be inside
__init__, right?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.
A major problem here is the circular includes of core's Task and client's Task.
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.
Then can we justify why we need two tasks? Like can we directly use core's Task?
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.
Core's tasks contains details that should be hidden from user. We could change the underlying implementation later. (E.g. Add a language column).