-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 SQL agent and Spider environment #1218
Conversation
Hi @byronxu99 Can you provide details about this SQL agent and Spider environment, as well as their benefits? The test dir is generally reserved for test scripts to be run by pytest or by users. New agents generally belong under agentchat/contrib. |
Hi @rickyloynd-microsoft I moved the agent into the notebook directory as an example, hopefully that's a better place. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1218 +/- ##
=======================================
Coverage 32.48% 32.48%
=======================================
Files 41 41
Lines 4907 4907
Branches 1120 1120
=======================================
Hits 1594 1594
Misses 3187 3187
Partials 126 126
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Yes, it's simpler. Is this PR just adding a notebook example of how to use SpiderEnv through function calling and SQL? The loop at the bottom of the nb would be convenient if it were inside a .py file, but in the nb it would be better to replace the for loop with a few example turns to illustrate and document the responses that are expected. The other notebooks are organized this way. Where is spider_env defined? Did that file get dropped from the PR? At the top of the PR, there should be some explanation of the benefits. ("Why are these changes needed?") |
Is this the Spider nl2sql benchmark? That would probably fit better under autogenbench |
@rickyloynd-microsoft @afourney The PR should now be ready to be merged. |
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.
Looks good!
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.
LGTM
Could you add a link in https://microsoft.github.io/autogen/docs/Examples ? That will help with the discoverability. |
I am okay with this notebook. However, the agent is designed for "Spider" benchmark, which is not used generally for actual product. For instance, when users want to use a SQL agent, they would not expect to give "reward" or "score" for the answer, as they may not even know the answer. So, I would suggest to include the name "spider" into the notebook file name. |
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.
LGTM now~ Thanks!
* add SQL agent and Spider environment * Make a notebook. Clean up environment. * add SQL agent and Spider environment * Make a notebook. Clean up environment. * clarify that sql agent is for spider environment * add link to notebook in docs * Update doc. --------- Co-authored-by: Wangda Zhang <[email protected]> Co-authored-by: Beibin Li <[email protected]> Co-authored-by: Chi Wang <[email protected]>
* add SQL agent and Spider environment * Make a notebook. Clean up environment. * add SQL agent and Spider environment * Make a notebook. Clean up environment. * clarify that sql agent is for spider environment * add link to notebook in docs * Update doc. --------- Co-authored-by: Wangda Zhang <[email protected]> Co-authored-by: Beibin Li <[email protected]> Co-authored-by: Chi Wang <[email protected]>
Why are these changes needed?
This PR adds a notebook that shows how to use Autogen for natural lanugage to SQL. It uses a text-to-SQL benchmark called Spider, which can be installed as a pip package.
Related issue number
None
Checks