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

Check writer pipeline #1201

Closed
wants to merge 32 commits into from
Closed

Check writer pipeline #1201

wants to merge 32 commits into from

Conversation

ivanleomk
Copy link
Collaborator

@ivanleomk ivanleomk commented Nov 21, 2024

Important

Integrate Writer's Palmyra-X-004 model with structured outputs, documentation, and tests, introducing WRITER_TOOLS mode.

  • Integration:
    • Add support for Writer's Palmyra-X-004 model in instructor.
    • Introduce from_writer() function in client_writer.py for Writer and AsyncWriter clients.
    • Add WRITER_TOOLS mode in mode.py.
  • Documentation:
    • Add writer.md for integration guide.
    • Update index.md to include Writer in supported providers.
    • Add blog post writer-support.md announcing the integration.
  • Testing:
    • Add tests for Writer integration in tests/llm/test_writer/.
    • Include tests for classification, sentiment analysis, entity extraction, and streaming.
  • Miscellaneous:
    • Update pyproject.toml to include writer-sdk as a dependency.
    • Add yanomaly to .authors.yml.

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

yanomaly and others added 30 commits November 11, 2024 14:11
@github-actions github-actions bot added dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files. labels Nov 21, 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 everything up to a0b8dc5 in 1 minute and 22 seconds

More details
  • Looked at 1766 lines of code in 28 files
  • Skipped 1 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. tests/llm/test_writer/test_format_common_models.py:159
  • Draft comment:
    The Jinja2 template syntax in the prompt variable is not being processed. Consider using a template engine to render the template before passing it to the messages parameter.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The test file test_writer/test_format_common_models.py contains a test function test_writer_format_list_of_strings that uses a Jinja2 template in a string. However, the template syntax is not being processed, which might lead to unexpected behavior or errors during test execution.
2. tests/llm/test_writer/evals/test_classification_enums.py:55
  • Draft comment:
    Assertions should include error messages for clarity. This applies to other test files as well.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The assertion statements in the test files lack error messages. Adding error messages will make it easier to understand the cause of a test failure.

Workflow ID: wflow_PEYsq5d0BKWE8gDc


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

Copy link

cloudflare-workers-and-pages bot commented Nov 21, 2024

Deploying instructor-py with  Cloudflare Pages  Cloudflare Pages

Latest commit: c2924c4
Status: ✅  Deploy successful!
Preview URL: https://2a657af9.instructor-py.pages.dev
Branch Preview URL: https://pipeline-check.instructor-py.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.

❌ Changes requested. Incremental review on 8652b9c in 1 minute and 6 seconds

More details
  • Looked at 116 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_6hAJvmbq98hrJ9SA


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -0,0 +1,57 @@
import enum
from itertools import product

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing import for List and Tuple. Add from typing import List, Tuple to avoid NameError.

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! Incremental review on c2924c4 in 22 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. tests/llm/test_writer/evals/test_sentiment_analysis.py:39
  • Draft comment:
    For compatibility with Python versions below 3.9, use List and Tuple from the typing module instead of list and tuple.
    model: str, data: List[Tuple[str, Sentiment]], mode: instructor.Mode
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The use of list and tuple in type hints is not compatible with Python versions below 3.9. The code should use List and Tuple from the typing module for compatibility.
2. tests/llm/test_writer/evals/test_sentiment_analysis.py:39
  • Draft comment:
    Use List and Tuple from typing for consistency with the rest of the codebase.
    model: str, data: List[Tuple[str, Sentiment]], mode: instructor.Mode
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The type hinting in the test function is inconsistent with the rest of the codebase, which uses List and Tuple from typing.

Workflow ID: wflow_IpzzC3GeWE5nF4kX


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

@ivanleomk ivanleomk closed this Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants