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

Fix examples #1077

Merged
merged 6 commits into from
Oct 16, 2024
Merged

Fix examples #1077

merged 6 commits into from
Oct 16, 2024

Conversation

ivanleomk
Copy link
Collaborator

@ivanleomk ivanleomk commented Oct 15, 2024

Important

This pull request updates GitHub Actions, documentation examples, and dependencies to enhance functionality and documentation clarity.

  • GitHub Actions:
    • Add system dependencies graphviz, libcairo2-dev, and xdg-utils in test_docs.yml.
  • Documentation Examples:
    • Update batch_job_oai.md to refine question generation example and output format.
    • Enhance extracting_tables.md with improved table extraction and markdown conversion examples.
    • Modify knowledge_graph.md to include iterative graph updates and visualization.
    • Update moderation.md to include detailed error handling examples.
  • Dependencies:
    • Add graphviz, sqlmodel, trafilatura, pydub, and datasets to pyproject.toml.
    • Update pyproject.toml to include new dependencies in test-docs group.

This description was created by Ellipsis for d4ad77c. 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 210836b in 28 seconds

More details
  • Looked at 585 lines of code in 6 files
  • Skipped 1 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. .github/workflows/test_docs.yml:18
  • Draft comment:
    Consider moving the 'Install system dependencies' step after the 'Set up Python' step to ensure the correct Python version is used for any Python-related dependencies.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds system dependencies installation in the GitHub Actions workflow, which is a good practice for ensuring the environment is set up correctly. However, the installation of system dependencies should ideally be done after setting up Python to ensure that the correct Python version is used for any Python-related dependencies.
2. docs/examples/batch_job_oai.md:68
  • Draft comment:
    The 'chain_of_thought' mentions details not included in the 'question' or 'answer'. Consider aligning the 'chain_of_thought' with the updated 'question' and 'answer' for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR modifies the question and answer in the example to be more concise. However, the chain_of_thought still mentions details that are not included in the question or answer, which might confuse readers.
3. docs/examples/extracting_tables.md:59
  • Draft comment:
    The code block from lines 59 to 149 is duplicated in multiple places. Consider refactoring to avoid redundancy and improve maintainability. This applies to other similar blocks in the file.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR includes multiple instances of duplicated code in the 'extracting_tables.md' file. This redundancy can lead to maintenance issues and should be refactored to improve code quality.
4. docs/examples/moderation.md:43
  • Draft comment:
    Ensure that the error message format is consistent with the actual output from Pydantic validation to avoid confusion.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR modifies the error messages in the moderation example to be more descriptive. However, the error messages should be consistent with the actual output from the Pydantic validation to avoid confusion.
5. pyproject.toml:102
  • Draft comment:
    New dependencies have been added, but there is no corresponding update to documentation or tests. Ensure that any changes in library code are reflected in the documentation and tests.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The PR introduces new dependencies in the pyproject.toml file, which are likely used in the examples. However, there is no update to the documentation or tests related to these dependencies. This is a violation of the rule that if library code changes, documentation and tests should be updated accordingly.

Workflow ID: wflow_LaYmWDn8W4EmDcWt


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

Copy link

cloudflare-workers-and-pages bot commented Oct 15, 2024

Deploying instructor-py with  Cloudflare Pages  Cloudflare Pages

Latest commit: d4ad77c
Status: ✅  Deploy successful!
Preview URL: https://51d9e996.instructor-py.pages.dev
Branch Preview URL: https://fix-examples.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 d4ad77c in 52 seconds

More details
  • Looked at 301 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. docs/examples/extracting_receipts.md:15
  • Draft comment:
    The import statement for model_validator is incorrect. It should be from pydantic import validator instead of from pydantic import model_validator. This mistake is repeated multiple times in the file.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. docs/examples/extracting_receipts.md:146
  • Draft comment:
    The model parameter in the extract function is inconsistent. It uses gpt-4 in one instance and gpt-4o in another. This inconsistency should be resolved.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The model parameter in the extract function is inconsistent. It uses gpt-4 in one instance and gpt-4o in another. This inconsistency should be resolved.
3. docs/examples/local_classification.md:43
  • Draft comment:
    The type: ignore comments are used multiple times in the file. Ensure these are necessary and not suppressing important type errors. This applies to lines 43, 49, and 57.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The type: ignore comments are used multiple times in the file. It's important to ensure that these are necessary and not suppressing important type errors.
4. docs/examples/mistral.md:30
  • Draft comment:
    The import statement for MistralClient has been changed to Mistral, which is correct according to the context provided. No issues here.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The import statement for MistralClient has been changed to Mistral, which is correct according to the context provided. No issues here.
5. docs/examples/sqlmodel.md:38
  • Draft comment:
    The create_hero function uses gpt-3.5-turbo, which might be outdated if the rest of the examples are using gpt-4. Consider updating for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The create_hero function uses gpt-3.5-turbo, which might be outdated if the rest of the examples are using gpt-4. Consider updating for consistency.

Workflow ID: wflow_IOjjyIf1Fwbpyb6l


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.

docs/examples/extracting_receipts.md Show resolved Hide resolved
@ivanleomk
Copy link
Collaborator Author

test_examples.py

Previous : 41 Failed, 36 Passed
Current: 29 Failed, 48 Passed

@ivanleomk
Copy link
Collaborator Author

Most changes are from poetry.lock and pyproject.toml updates

@jxnl jxnl merged commit 356c317 into main Oct 16, 2024
14 of 15 checks passed
@jxnl jxnl deleted the fix-examples branch October 16, 2024 03:36
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