Skip to content
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

refactor: create_tool_from_function + tool decorator #8697

Merged
merged 7 commits into from
Jan 10, 2025

Conversation

anakin87
Copy link
Member

@anakin87 anakin87 commented Jan 9, 2025

Related Issues

Since we are introducing some subclasses of Tool (ComponentTool, OpenAPITool in the future, ...), the Tool.from_function class method becomes inappropriate. These subclasses would inherit the method, potentially leading to unintended behaviors (e.g. ComponentTool.from_function makes no sense).

To address this, I'm proposing a replacement: create_tool_from_function utility function (+ a tool decorator).

Proposed Changes:

  • Replace the Tool.from_function class method with a create_tool_from_function utility function.
  • Add a simple tool decorator, to simplify programmatic use cases

Examples

from typing import Annotated, Literal
from haystack.tools import create_tool_from_function

def get_weather(
    city: Annotated[str, "the city for which to get the weather"] = "Munich",
    unit: Annotated[Literal["Celsius", "Fahrenheit"], "the unit for the temperature"] = "Celsius"):
    '''A simple function to get the current weather for a location.'''
    return f"Weather report for {city}: 20 {unit}, sunny"

tool = create_tool_from_function(get_weather)

print(tool)
>>> Tool(name='get_weather', description='A simple function to get the current weather for a location.',
>>> parameters={
>>> 'type': 'object',
>>> 'properties': {...},
>>> function=<function get_weather at 0x7f7b3a8a9b80>)
from typing import Annotated, Literal
from haystack.tools import tool
@tool
def get_weather(
    city: Annotated[str, "the city for which to get the weather"] = "Munich",
    unit: Annotated[Literal["Celsius", "Fahrenheit"], "the unit for the temperature"] = "Celsius"):
    '''A simple function to get the current weather for a location.'''
    return f"Weather report for {city}: 20 {unit}, sunny"

print(get_weather)
>>> Tool(name='get_weather', description='A simple function to get the current weather for a location.',
>>> parameters={
>>> 'type': 'object',
>>> 'properties': {...},
>>> function=<function get_weather at 0x7f7b3a8a9b80>)

How did you test it?

CI, new tests

Notes for the reviewer

I also reorganized the code by moving functionalities into separate modules.

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@github-actions github-actions bot added topic:tests type:documentation Improvements on the docs labels Jan 9, 2025
@anakin87 anakin87 self-assigned this Jan 9, 2025
@anakin87 anakin87 marked this pull request as ready for review January 9, 2025 17:54
@anakin87 anakin87 requested review from a team as code owners January 9, 2025 17:54
@anakin87 anakin87 requested review from dfokina, julian-risch and vblagoje and removed request for a team January 9, 2025 17:54
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 12695370849

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 91.099%

Totals Coverage Status
Change from base Build 12694180152: 0.005%
Covered Lines: 8659
Relevant Lines: 9505

💛 - Coveralls

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 12695378991

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 91.099%

Totals Coverage Status
Change from base Build 12694180152: 0.005%
Covered Lines: 8659
Relevant Lines: 9505

💛 - Coveralls

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 12695708412

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 91.099%

Totals Coverage Status
Change from base Build 12694180152: 0.005%
Covered Lines: 8659
Relevant Lines: 9505

💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jan 9, 2025

Pull Request Test Coverage Report for Build 12707767349

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 91.099%

Totals Coverage Status
Change from base Build 12694180152: 0.005%
Covered Lines: 8659
Relevant Lines: 9505

💛 - Coveralls

Copy link
Member

@vblagoje vblagoje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a small comment but more importantly I think we need to clarify and document what parameter inputs types are supported for the function passed to create_tool_from_function and tool decorator.

haystack/tools/from_function.py Show resolved Hide resolved
@anakin87 anakin87 requested a review from vblagoje January 10, 2025 10:37
Copy link
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 👍 @tool decorator is an interesting idea!

@anakin87 anakin87 merged commit 08cf09f into main Jan 10, 2025
19 checks passed
@anakin87 anakin87 deleted the from-function-refactoring branch January 10, 2025 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants