-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 method support for tool decorator #627
base: main
Are you sure you want to change the base?
Conversation
The original tool decorator doesn't work on class methods because it unconditionally adds a self parameter treats method as if it were a plain function. That does not play nicely with Python’s bound-method behavior. This inspects the signature to prevent double-binding of self.
src/smolagents/tools.py
Outdated
new_signature = original_signature.replace(parameters=new_parameters) | ||
simple_tool.forward.__signature__ = new_signature | ||
|
||
@functools.wraps(tool_function) |
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.
Aren't you reinventing inspect.ismethod
and inspect.isfunction
? First would correctly detect classmethod and just a method, the second will capture staticmethod and a regular function.
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.
Didn’t know about that one, thanks. Will confirm with tests today
Co-authored-by: Alex <sysradium@users.noreply.github.com>
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.
Thanks. Maybe creating some tests?
Will do |
@albertvillanova @sysradium I added tests and documentation. Can you run the workflow for me? |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
…orators Merge main with test fixes
@albertvillanova Do you mind triggering the tests one more time? |
Any update on this? |
Unfortunately I don't have rights to run the workflow : / |
This PR address #574 and allows the tool decorator to work with class methods.
The original tool decorator doesn't work on class methods because it unconditionally adds a self parameter and treats methods as if it were a plain function. That does not play nicely with Python’s bound-method behavior. This inspects the signature to prevent double-binding of self.