-
Notifications
You must be signed in to change notification settings - Fork 665
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
Fixes gather_with_concurrency
typing
#714
Conversation
@@ -650,7 +650,7 @@ async def aget_evidence( | |||
for _, llm_result in results: | |||
session.add_tokens(llm_result) | |||
|
|||
session.contexts += [r for r, _ in results if r is not None] | |||
session.contexts += [r for r, _ in results] |
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.
This is being discussed in #687. However, the typing change didn't fix the mypy error saying that r is not None
is a always True statement
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.
Cannot do this I think. You can get a None
back I believe
gather_with_concurrency
typinggather_with_concurrency
typing
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.
@maykcaldas looks like pydantic==2.10.1
is out, can you rebase or merge main for the updated uv.lock
…penAI connection error
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.
LGTM from my side. Looks like Andrew has an outstanding comment
Changes the type hint of
gather_with_concurrency
fromgather_with_concurrency(n: int, coros: list[Coroutine]) -> list[Any]:
to
Also pinned pydantic version to <2.10 until they release the v2.10 bugfix.