-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add convenience method to use LangChain community tools #1832
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
Conversation
…checked, not executed
# Conflicts: # pydantic_ai_slim/pydantic_ai/_output.py # pydantic_ai_slim/pydantic_ai/tools.py
…er to rerun when the length of the example changes
This does work however the style and approach are very much open for discussion. In particular: * this creates a function just so it can be inspected, as part of that it has a very bad mapping from json schema types back to python. * the Tool class current has no base class that defines the behaviour. If it did then creating a separate class that doesn't need the round trip might be appropriate
I don't know how you want to handle the typing of the tool to pass in. It feels excessive to make pydantic_ai require langchain for type checking. Added optional group of langchain and attempted import in |
This file contains the code for |
If the optional group is not installed then a protocol is used to define the tool type used.
PR Change SummaryIntroduced a new installation option for the LangChain library in the documentation.
Modified Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
There are a bunch of unstable pre-commit hooks that I can't satisfy. The format one in particular wants to change unrelated code.
Just makes the checks unhappy
https://smolagents.org/docs/tools-of-smolagents-in-depth-guide/#3-toc-title They write well and in a positive way.
Think I have addressed all your comments. I have added a section to the tools documentation. It's heavily based on the smolagents documentation for the same functionality which you can view here. I updated the example to be something that requires a web search to answer correctly. |
"""Creates a Pydantic tool from a function and a JSON schema. | ||
Args: | ||
function: The function to call |
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.
@matthewfranglen This discussion still needs to be resolved!
def proxy(*args: Any, **kwargs: Any) -> str: | ||
assert not args, 'This should always be called with kwargs' | ||
kwargs = defaults | kwargs | ||
return langchain_tool.run(kwargs) |
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.
Is there a validation error or something this could raise that we could wrap in ModelRetry
, like we do with our own validation errors?
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 tried writing a tool that immediately throws an exception and invoking it using a langchain agent. The agent invocation appeared to hang. Maybe this is something that could be further investigated in future.
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.
OK, just wanted to make sure there are no obvious exceptions for us to catch and wrap and reraise here.
docs/tools.md
Outdated
|
||
## Use LangChain Tools {#langchain-tools} | ||
|
||
We love LangChain and think it has a very compelling suite of tools. To import a tool from LangChain, use the `from_langchain()` method. |
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.
With all due respect to the LangChain team and product, we are competitors so I'd like this to be a bit more neutral :) Something like:
If you'd like to use a tool from LangChain's community tool library with PydanticAI, you can use the
Tool.from_langchain
convenience method. Note that PydanticAI will not validate the arguments in this case -- it's up to the model to provide arguments matching the schema specified by the LangChain tool, and up to the LangChain tool to raise an error if the arguments are invalid.
I'd expect "community tool library" to be a link.
If we manage to wrap LangChain tool errors in ModelRetry
, we may be able to soften the language in the second sentence.
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 we should mention that it's a good way to transition to PydanticAI.
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.
If you want to propose alternative text then please do. Since the documentation is presenting a specific view of the pydantic product that you want to preserve I think it would be faster for all involved if any documentation changes were provided verbatim.
The langchain-community dependency is not installed so it cannot be run
I am fed up with this linter. It's one import. It still fails. It doesn't say how to fix it and there isn't a way to automatically apply the fix.
) | ||
|
||
@classmethod | ||
def from_langchain(cls, langchain_tool: LangChainTool) -> Self: |
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.
What do you think about moving this off of Tool
and into a new module like pydantic_ai.ext.langchain
? We will likely have more wrappers and convenience methods for other frameworks in the future, so my colleague @Viicos correctly pointed out that we should probably start moving them into their own namespace already instead of cluttering up the regular public classes.
This wouldn't need to be class method in that case, I imagine:
from pydantic_ai import Agent
from pydantic_ai.ext.langchain import langchain_tool
from langchain_community.tools import DuckDuckGoSearchRun
agent = Agent(..., tools=[langchain_tool(DuckDuckGoSearchRun])
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've moved it and named the function from_langchain_tool
.
def proxy(*args: Any, **kwargs: Any) -> str: | ||
assert not args, 'This should always be called with kwargs' | ||
kwargs = defaults | kwargs | ||
return langchain_tool.run(kwargs) |
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.
OK, just wanted to make sure there are no obvious exceptions for us to catch and wrap and reraise here.
References #1831
This does work however the style and approach are very much open for discussion. In particular:
pydantic_ai.tool.Tool
class has no base class that describes the desired behaviour.pydantic_ai.tool.Tool
class.To make my PR simple I have introduced a minimal change. I think it would be good to discuss a more maintainable and extensible approach.