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(client,function_calls,process_response,test_new_client): enhance create method to handle multiple responses #578

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jxnl
Copy link
Owner

@jxnl jxnl commented Apr 8, 2024

Ellipsis 🚀 This PR description was created by Ellipsis for commit bfa4ca1.

Summary:

This PR enhances the create method to handle multiple responses from the OpenAI API, updates various methods in function_calls.py and process_response.py to handle multiple responses, and includes a test for multiple responses in test_new_client.py.

Key points:

  • Overloaded create method in client.py to handle multiple responses.
  • Updated handle_kwargs method in client.py to include n parameter in kwargs.
  • Updated function_calls.py to handle multiple responses in parse_functions, parse_tools, and parse_json methods.
  • Updated process_response.py to attach raw response to each model in case of IterableBase response model.
  • Updated test_new_client.py to include a test for multiple responses.

Generated with ❤️ by ellipsis.dev

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Apr 8, 2024
@jxnl jxnl mentioned this pull request Apr 8, 2024
@ellipsis-dev ellipsis-dev bot changed the title ... feat(client,function_calls,process_response,test_new_client): enhance create method to handle multiple responses Apr 8, 2024
Copy link

cloudflare-pages bot commented Apr 8, 2024

Deploying instructor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 712f15a
Status: ✅  Deploy successful!
Preview URL: https://53be96dd.instructor.pages.dev
Branch Preview URL: https://support-n.instructor.pages.dev

View logs

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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!

  • Reviewed the entire pull request up to bfa4ca1
  • Looked at 364 lines of code in 4 files
  • Took 1 minute and 32 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 4 additional comments because they didn't meet confidence threshold of 85%.
1. instructor/client.py:65:
  • Assessed confidence : 10%
  • Comment:
    The changes in this file look good. The support for multiple responses (n > 1) has been added correctly in the create method of both Instructor and AsyncInstructor classes. The code is clear and follows the best practices.
  • Reasoning:
    The PR author has added support for multiple responses (n > 1) in the create method of the Instructor and AsyncInstructor classes. The changes seem to be logically correct and follow the best practices. The author has also updated the tests to cover the new functionality.
2. instructor/function_calls.py:76:
  • Assessed confidence : 10%
  • Comment:
    The changes in this file look good. The from_response method in the OpenAISchema class has been updated correctly to handle multiple responses. The code is clear and follows the best practices.
  • Reasoning:
    The PR author has updated the from_response method in the OpenAISchema class to handle multiple responses. The changes seem to be logically correct and follow the best practices.
3. instructor/process_response.py:153:
  • Assessed confidence : 10%
  • Comment:
    The changes in this file look good. The process_response and process_response_async methods have been updated correctly to handle multiple responses. The code is clear and follows the best practices.
  • Reasoning:
    The PR author has updated the process_response and process_response_async methods in the process_response.py file to handle multiple responses. The changes seem to be logically correct and follow the best practices.
4. tests/test_new_client.py:53:
  • Assessed confidence : 10%
  • Comment:
    The changes in this file look good. The tests have been updated correctly to cover the new functionality of handling multiple responses. The code is clear and follows the best practices.
  • Reasoning:
    The PR author has updated the tests to cover the new functionality of handling multiple responses. The changes seem to be logically correct and follow the best practices.

Workflow ID: wflow_wIPhyK9XK3e85BBb


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

@jxnl
Copy link
Owner Author

jxnl commented May 8, 2024

@ellipsis-dev can you fix the merge conflicts

Copy link
Contributor

ellipsis-dev bot commented May 8, 2024

Could you please specify which files have conflicts and what the conflicts are? This will help me resolve the merge conflicts more effectively. Thank you.


Responding to this comment by @jxnl. For more information about Ellipsis, check the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant