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(instructor): introduce ANTHROPIC_JSON mode #542

Merged
merged 9 commits into from
Mar 29, 2024
Merged

feat(instructor): introduce ANTHROPIC_JSON mode #542

merged 9 commits into from
Mar 29, 2024

Conversation

jxnl
Copy link
Owner

@jxnl jxnl commented Mar 29, 2024

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

Summary:

This PR introduces a new ANTHROPIC_JSON mode in the Instructor library, updates relevant files and tests, modifies GitHub workflows in .github/workflows/test.yml, and updates the poetry.lock file.

Key points:

  • Introduced a new ANTHROPIC_JSON mode in the Instructor library in instructor/mode.py
  • Updated instructor/function_calls.py and instructor/process_response.py to handle the new mode
  • Updated documentation in docs/blog/posts/anthropic.md and docs/concepts/patching.md
  • Updated tests in tests/anthropic/test_simple.py
  • Removed examples/classification/test_run.py and tests/anthropic/test_anthropic.py
  • Modified GitHub workflows in .github/workflows/test.yml to include anthropic dependency
  • Updated poetry.lock file

Generated with ❤️ by ellipsis.dev

@jxnl jxnl marked this pull request as ready for review March 29, 2024 02:35
Copy link

cloudflare-pages bot commented Mar 29, 2024

Deploying instructor with  Cloudflare Pages  Cloudflare Pages

Latest commit: abfded0
Status:🚫  Build failed.

View logs

@ellipsis-dev ellipsis-dev bot changed the title ... feat(instructor): introduce ANTHROPIC_JSON mode Mar 29, 2024
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 ab1b10b
  • Looked at 394 lines of code in 8 files
  • Took 2 minutes and 11 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 0 additional comments because they didn't meet confidence threshold of 85%.

Workflow ID: wflow_fftB6sZVemh5d4kU


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

@roodboi
Copy link

roodboi commented Mar 29, 2024

why not just use MD_JSON vs adding a new mode

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.

❌ Changes requested.

  • Performed an incremental review on ddcf7bf
  • Looked at 98 lines of code in 3 files
  • Took 1 minute and 17 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 0 additional comments because they didn't meet confidence threshold of 85%.

Workflow ID: wflow_VtT8Hg3zt5QJPejC


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.

@@ -20,15 +20,15 @@ jobs:
uses: actions/setup-python@v4
with:
python-version: 3.11
Copy link
Contributor

Choose a reason for hiding this comment

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

Python 3.11 is not yet released. Please use a stable version of Python.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, @jxnl - great example of how the training data extension stuff in GPT4 isn't perfect yet.

@@ -22,8 +22,8 @@ jobs:
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Python 3.11 is not yet released. Please use a stable version of Python.

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!

  • Performed an incremental review on d4a3fdb
  • Looked at 191 lines of code in 11 files
  • Took 3 minutes and 7 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 7 additional comments because they didn't meet confidence threshold of 85%.
1. .github/workflows/test.yml:36:
  • Assessed confidence : 50%
  • Comment:
    The poetry install command has been updated to include the anthropic option. Please ensure that this option is necessary and that it does not introduce any potential issues or conflicts with other dependencies.
  • Reasoning:
    The PR introduces a new mode ANTHROPIC_JSON in instructor/mode.py and updates the instructor/function_calls.py and instructor/process_response.py to handle the new mode. The PR also updates the documentation and tests to reflect these changes. However, the PR does not provide the actual changes in the mentioned files. It would be necessary to review these changes to ensure that the new mode is implemented correctly and that the tests cover the new functionality. Additionally, the PR removes some files, but it does not provide a reason for these removals. It would be important to understand why these files were removed and whether their removal could impact the functionality of the software.
2. tests/llm/test_openai/evals/test_classification_enums.py:11:
  • Assessed confidence : 50%
  • Comment:
    The import statement has been updated. Please ensure that this change does not break the tests and that all necessary modules are imported correctly.
  • Reasoning:
    The PR includes changes to the test files, renaming them and updating the imports. However, without the actual changes in the test files, it's hard to determine if the tests are updated correctly to cover the new ANTHROPIC_JSON mode. It would be necessary to review these changes to ensure that the tests are updated correctly.
3. tests/llm/test_openai/evals/test_classification_literals.py:11:
  • Assessed confidence : 50%
  • Comment:
    The import statement has been updated. Please ensure that this change does not break the tests and that all necessary modules are imported correctly.
  • Reasoning:
    The PR includes changes to the test files, renaming them and updating the imports. However, without the actual changes in the test files, it's hard to determine if the tests are updated correctly to cover the new ANTHROPIC_JSON mode. It would be necessary to review these changes to ensure that the tests are updated correctly.
4. tests/llm/test_openai/evals/test_entities.py:11:
  • Assessed confidence : 50%
  • Comment:
    The import statement has been updated. Please ensure that this change does not break the tests and that all necessary modules are imported correctly.
  • Reasoning:
    The PR includes changes to the test files, renaming them and updating the imports. However, without the actual changes in the test files, it's hard to determine if the tests are updated correctly to cover the new ANTHROPIC_JSON mode. It would be necessary to review these changes to ensure that the tests are updated correctly.
5. tests/llm/test_openai/test_modes.py:11:
  • Assessed confidence : 50%
  • Comment:
    The import statement has been updated. Please ensure that this change does not break the tests and that all necessary modules are imported correctly.
  • Reasoning:
    The PR includes changes to the test files, renaming them and updating the imports. However, without the actual changes in the test files, it's hard to determine if the tests are updated correctly to cover the new ANTHROPIC_JSON mode. It would be necessary to review these changes to ensure that the tests are updated correctly.
6. tests/llm/test_openai/test_stream.py:11:
  • Assessed confidence : 50%
  • Comment:
    The import statement has been updated. Please ensure that this change does not break the tests and that all necessary modules are imported correctly.
  • Reasoning:
    The PR includes changes to the test files, renaming them and updating the imports. However, without the actual changes in the test files, it's hard to determine if the tests are updated correctly to cover the new ANTHROPIC_JSON mode. It would be necessary to review these changes to ensure that the tests are updated correctly.
7. tests/llm/test_openai/test_validators.py:11:
  • Assessed confidence : 50%
  • Comment:
    The import statement has been updated. Please ensure that this change does not break the tests and that all necessary modules are imported correctly.
  • Reasoning:
    The PR includes changes to the test files, renaming them and updating the imports. However, without the actual changes in the test files, it's hard to determine if the tests are updated correctly to cover the new ANTHROPIC_JSON mode. It would be necessary to review these changes to ensure that the tests are updated correctly.

Workflow ID: wflow_70cupd6tCyCrKMl6


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

@jxnl
Copy link
Owner Author

jxnl commented Mar 29, 2024

why not just use MD_JSON vs adding a new mode

i dont have polyglot,

so i have to handle system message, and getting the text out

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!

  • Performed an incremental review on 31810fa
  • Looked at 301 lines of code in 1 files
  • Took 3 minutes and 28 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 0 additional comments because they didn't meet confidence threshold of 85%.

Workflow ID: wflow_YzRJWAj2fQyXhRZ3


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

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!

  • Performed an incremental review on b887b2e
  • Looked at 12 lines of code in 1 files
  • Took 1 minute and 29 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 85%.
1. .github/workflows/test_docs.yml:39:
  • Assessed confidence : 80%
  • Comment:
    Please ensure that the test path 'tests/llm/test_openai/docs' exists and contains the relevant tests for the documentation.
  • Reasoning:
    The test path in the 'Run tests' step of the GitHub workflow has been changed. I need to check if the new path 'tests/llm/test_openai/docs' exists in the codebase and contains the relevant tests.

Workflow ID: wflow_R8kDzfj7nyTmJckF


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

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!

  • Performed an incremental review on 492f671
  • Looked at 124 lines of code in 4 files
  • Took 2 minutes and 11 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 85%.
1. docs/concepts/patching.md:87:
  • Assessed confidence : 100%
  • Grade: 0%
  • Comment:
    The PR description mentions the introduction of a new ANTHROPIC_JSON mode, but this is not visible in the diff. Please ensure that this new mode has been implemented correctly in instructor/mode.py, instructor/function_calls.py, and instructor/process_response.py.
  • Reasoning:
    The PR introduces a new ANTHROPIC_JSON mode, but I don't see any changes in the codebase that reflect this. The PR description mentions updates to instructor/mode.py, instructor/function_calls.py, and instructor/process_response.py, but these are not visible in the diff. I need to check these files in the codebase to ensure the new mode has been implemented correctly.

Workflow ID: wflow_CUbyr40lQp17KkQO


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

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!

  • Performed an incremental review on abfded0
  • Looked at 22 lines of code in 1 files
  • Took 2 minutes and 8 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 85%.
1. .github/workflows/test.yml:39:
  • Assessed confidence : 100%
  • Grade: 0%
  • Comment:
    The pytest command is excluding tests related to anthropic. If there are new tests related to the ANTHROPIC_JSON mode, they might not be executed. Please ensure that the new tests are included in the CI/CD pipeline.
  • Reasoning:
    The PR mentions that a new mode ANTHROPIC_JSON is introduced and the tests are updated. However, in the GitHub workflow file, the pytest command is excluding tests related to anthropic. This might lead to the new tests not being executed as part of the CI/CD pipeline.

Workflow ID: wflow_O2yxfoFByzpF4jM4


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

@jxnl jxnl merged commit 5c2496c into main Mar 29, 2024
6 of 13 checks passed
@jxnl jxnl deleted the anthropic-json branch March 29, 2024 03:10
PrathamSoni pushed a commit to EndexAI/instructor that referenced this pull request Apr 10, 2024
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.

None yet

3 participants