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

Improve static tools initialization safety #324

Merged

Conversation

kingdomad
Copy link
Contributor

Create a copy of static_tools to avoid modifying the original object.

@kingdomad
Copy link
Contributor Author

@aymeric-roucher Is that ok?

Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

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

Thanks.

With your contribution, the originally passed static_tools dict will not be changed by evaluate_python_code function. Specifically, in this line:

static_tools["final_answer"] = final_answer

Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

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

It breaks test: test_function_persistence_across_steps

@kingdomad
Copy link
Contributor Author

It breaks test: test_function_persistence_across_steps

How do I obtain this test code?

@kingdomad
Copy link
Contributor Author

I have fixed the issue. Since custom_tools needs to pass tool objects that are dynamically created during code execution, it is necessary to allow modifications to the original object.

Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

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

Thank you.

@albertvillanova albertvillanova merged commit b351a8c into huggingface:main Jan 23, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants