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

Edits to batch.py to support Anthropic batch API #1193

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

economy
Copy link
Contributor

@economy economy commented Nov 19, 2024

I noticed the structure of the JSONL file produced by create_from_messages was incorrect for the Anthropic API. This change allows submission of batch jobs to Anthropic by creating the correct JSONL structure


Important

create_from_messages in batch.py now supports Anthropic batch API by correctly formatting JSONL files.

  • Behavior:
    • create_from_messages in batch.py now supports Anthropic batch API by formatting JSONL files correctly.
    • Uses a conditional check for "claude" in the model name to determine if Anthropic format is needed.
  • Implementation:
    • For Anthropic, constructs a JSON object with custom_id, params including model, max_tokens, temperature, and messages.
    • Writes formatted JSONL to file for Anthropic and OpenAI formats.
  • Misc:
    • Minor restructuring of file writing logic in create_from_messages.

This description was created by Ellipsis for b69522c. It will automatically update as commits are pushed.

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 everything up to b69522c in 26 seconds

More details
  • Looked at 53 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. instructor/batch.py:126
  • Draft comment:
    The logic use_anthropic = "claude" in model.lower() is fragile. Consider using a more robust method to determine the API format, such as a configuration or a mapping of model names to formats.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code for handling the Anthropic format in create_from_messages is correct, but the logic for determining the format based on the model name is fragile. It assumes that any model name containing 'claude' should use the Anthropic format, which might not be future-proof or accurate for all cases.
2. instructor/batch.py:157
  • Draft comment:
    The logic use_anthropic = "claude" in model.lower() is fragile. Consider using a more robust method to determine the API format, such as a configuration or a mapping of model names to formats.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code for handling the Anthropic format in create_from_messages is correct, but the logic for determining the format based on the model name is fragile. It assumes that any model name containing 'claude' should use the Anthropic format, which might not be future-proof or accurate for all cases.
3. instructor/batch.py:129
  • Draft comment:
    The logic for writing messages to a file is repeated for both Anthropic and OpenAI formats. Consider refactoring to eliminate redundancy and adhere to the DRY principle.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The code has repeated logic for handling the writing of messages to a file, which violates the DRY principle.
4. instructor/batch.py:131
  • Draft comment:
    The JSON parsing logic here is complex and could benefit from comments explaining the structure being parsed. This applies to similar logic on line 149 as well.
  • Reason this comment was not posted:
    Confidence changes required: 70%
    The code has multiple instances of complex JSON parsing logic that could benefit from comments for clarity.

Workflow ID: wflow_8B7rKNEOkfw2TTg7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@jxnl jxnl merged commit 06cdc13 into instructor-ai:main Nov 22, 2024
8 of 13 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