-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add duckduckgo search videos images news tools #3552
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?
Add duckduckgo search videos images news tools #3552
Conversation
DouweM
left a comment
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.
Thank! In addition to the comments I left, we need tests.
| The [`duckduckgo_images_search_tool`][pydantic_ai.common_tools.duckduckgo.duckduckgo_images_search_tool] searches for images with support for filtering by size, color, type, layout, and license. | ||
|
|
||
| ```py {title="duckduckgo_images.py"} | ||
| --8<-- "examples/pydantic_ai_examples/duckduckgo_images.py" |
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.
These example files don't exist; can you please use inline code?
Also, I don't think we need a separate section for each, the main section can link to each of the functions, and a single example can use 2 to illustrate the point.
| return videos_ta.validate_python(results) | ||
|
|
||
|
|
||
| def duckduckgo_videos_search_tool(duckduckgo_client: DDGS | None = None, max_results: int | None = None): |
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'd rather call the method and classes "video search" and "image search" without the extra s
| """The video content URL.""" | ||
| description: str | ||
| """The video description.""" | ||
| duration: str |
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.
Are these really all string fields?
| self, | ||
| query: str, | ||
| region: str = 'us-en', | ||
| safesearch: Literal['on', 'moderate', 'off'] = 'moderate', |
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 don't think safesearch and region should be provided by the LLM, can they be fields on the class?
| | None = None, | ||
| type_image: Literal['photo', 'clipart', 'gif', 'transparent', 'line'] | None = None, | ||
| layout: Literal['Square', 'Tall', 'Wide'] | None = None, | ||
| license_image: Literal['any', 'Public', 'Share', 'ShareCommercially', 'Modify', 'ModifyCommercially'] |
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.
Same as up; all of these seem like options the developer should pick, not the LLM via tool call args
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.
Instead of hard-coding types, are there types on the DDGS package we could use?
| """ | ||
| return Tool[Any]( | ||
| DuckDuckGoImagesSearchTool(client=duckduckgo_client or DDGS(), max_results=max_results).__call__, | ||
| name='duckduckgo_images', |
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.
Tool names should have search in them
Closes #3135