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

feat: support rest-style docstrings when loading tools from function #9004

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LastRemote
Copy link
Contributor

@LastRemote LastRemote commented Mar 10, 2025

Related Issues

N/A

Proposed Changes:

Supports ReST-style docstrings when loading a tool from function.
The docstring will be automatically parsed for function-level descriptions and parameter descriptions.
It's a small feature due to the fact that I was too lazy to write parameter annotations in class and instance methods.

How did you test it?

Added a new unit test for it.

Notes for the reviewer

N/A

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

@LastRemote LastRemote requested review from a team as code owners March 10, 2025 07:58
@LastRemote LastRemote requested review from dfokina and anakin87 and removed request for a team March 10, 2025 07:58
@LastRemote
Copy link
Contributor Author

P.S. Parameter and return type definitions in ReST docstrings are currently ignored to avoid things being too complicated.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 13759572106

Details

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

Totals Coverage Status
Change from base Build 13718432299: 0.03%
Covered Lines: 9718
Relevant Lines: 10802

💛 - Coveralls

Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

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

Hello!

When I originally developed the create_tool_from_function utility function, I also considered docstring parsing. However, I ultimately chose to use Annotated, which felt more robust and simpler.

I can see how the feature you're proposing could be valuable for some users, and I appreciate the effort you've put into this PR.

As framework maintainers, we need to strike a balance between introducing new features and keeping the codebase simple and maintainable. With that in mind, I'd suggest opening a GitHub issue, discussion, or Discord thread to gather feedback and see how much interest there is from the community (Unfortunately, I don't have much time to do this myself at the moment).

If there's significant interest, we can revisit this PR and explore ways to merge or refine it.

Thanks again for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants