-
Notifications
You must be signed in to change notification settings - Fork 511
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
Add GraphRun object to make use of next
more ergonomic
#833
base: main
Are you sure you want to change the base?
Conversation
next
more ergonomic
6ce755e
to
571d805
Compare
1443b49
to
c4e9180
Compare
571d805
to
04fc74c
Compare
39a6009
to
2621543
Compare
873fe35
to
e4d384a
Compare
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.
overall looks good, but obviously needs more work, and will conflict with #824.
aa74a7c
to
bc681bc
Compare
17409cc
to
0b93143
Compare
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 style guide flagged several spelling errors that seemed like false positives. We skipped posting inline suggestions for the following words:
- [Ii]terable
- [Aa]sync
- [Cc]oroutine
- [Dd]eps
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 think this is looking good.
I'm still a bit lost about iterating combined with streaming.
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.
should this be public since the nodes you get from iterating are now kind of public?
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 think it's enough to leave the module private and re-export the node classes, but I'm also okay to make the module public.
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 think I've done the re-export thing now, and it works for me in pycharm, but it might be worth double-checking on your end
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 style guide flagged several spelling errors that seemed like false positives. We skipped posting inline suggestions for the following words:
- [Ii]nherited_members
Docs Preview
|
6460a73
to
241f179
Compare
241f179
to
c7ab89f
Compare
# Re-exporting like this improves auto-import behavior in PyCharm | ||
capture_run_messages = _agent_graph.capture_run_messages | ||
EndStrategy = _agent_graph.EndStrategy | ||
HandleResponseNode = _agent_graph.HandleResponseNode | ||
ModelRequestNode = _agent_graph.ModelRequestNode | ||
UserPromptNode = _agent_graph.UserPromptNode |
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.
@samuelcolvin let me know if you think there's a better way to do this. (I tried other things I could think of and this worked best.)
This is the result of trying to reduce the amount of stuff happening in
Graph.run
that wasn't just innext
.It's now in a pretty good state, it does the thing we discussed of implementing
__await__
,__anext__
, andasync def next
to provide a way to async iterate over the result of callingagent.run
, hijack the execution withnext
, or justawait
it to get the output without iteration.