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

Tentative Fix for Mistral Fix #985

Merged
merged 7 commits into from
Sep 18, 2024
Merged

Tentative Fix for Mistral Fix #985

merged 7 commits into from
Sep 18, 2024

Conversation

ivanleomk
Copy link
Collaborator

@ivanleomk ivanleomk commented Sep 8, 2024

This seems to fix the issue with mistral - we just need to set the new dependency to their new client version.

import os
from pydantic import BaseModel, Field
from typing import List
from mistralai import Mistral
from instructor import from_mistral, Mode
from asyncio import run


class UserDetails(BaseModel):
    name: str
    age: int


# enables `response_model` in chat call
client = Mistral(api_key=os.environ["MISTRAL_API_KEY"])

instructor_client = from_mistral(
    client=client,
    model="mistral-large-latest",
    mode=Mode.MISTRAL_TOOLS,
)

🚀 This description was created by Ellipsis for commit 64111c7

fix: update Mistral client version and add async support

Summary:

Fix Mistral client issue by updating client version, adding async support, and updating dependencies.

Key points:

  • Fix Mistral client issue by updating to new client version.
  • Add async support in from_mistral() in client_mistral.py.
  • Introduce use_async parameter for sync/async operations.
  • Use client.chat.complete for sync and client.chat.complete_async for async operations.
  • Update mistralai to ^1.0.3, openai to ^1.45.0, and anthropic to ^0.34.0 in pyproject.toml.
  • Minor import statement changes in client_vertexai.py and utils.py.

Generated with ❤️ by ellipsis.dev

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. Reviewed everything up to ad7482b in 10 seconds

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

Workflow ID: wflow_VdcR91DiSFwFC2KT


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.

**kwargs: Any,
) -> instructor.Instructor | instructor.AsyncInstructor:
assert mode in {
instructor.Mode.MISTRAL_TOOLS,
}, "Mode be one of {instructor.Mode.MISTRAL_TOOLS}"

assert isinstance(
client, (mistralai.client.MistralClient, mistralaiasynccli.MistralAsyncClient)
), "Client must be an instance of mistralai.client.MistralClient or mistralai.async_cli.MistralAsyncClient"
client, (Mistral)
Copy link
Contributor

Choose a reason for hiding this comment

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

The isinstance check should use a tuple for the second argument. Use (Mistral,) instead of (Mistral). This ensures it's treated as a tuple.

Suggested change
client, (Mistral)
client, (Mistral,)

Copy link

cloudflare-workers-and-pages bot commented Sep 8, 2024

Deploying instructor with  Cloudflare Pages  Cloudflare Pages

Latest commit: c9461b0
Status: ✅  Deploy successful!
Preview URL: https://477daf5f.instructor.pages.dev
Branch Preview URL: https://mistral-fix.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! Incremental review on 754f707 in 13 seconds

More details
  • Looked at 36 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. instructor/client_mistral.py:36
  • Draft comment:
    The assertion message is unclear. Consider rephrasing it for clarity.
    }, "Mode must be one of {instructor.Mode.MISTRAL_TOOLS}"
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The assertion message on line 36 is missing a verb, which makes it unclear. It should be corrected for better readability.

Workflow ID: wflow_MWhajUgEn6u3ycNi


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

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 0ceb7b7 in 12 seconds

More details
  • Looked at 27 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. instructor/client_vertexai.py:9
  • Draft comment:
    Ensure that removing # type: ignore from the jsonref import does not introduce type checking issues. This change is also present in instructor/utils.py.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The import statement for jsonref was changed to remove the # type: ignore comment. This change was made in two files, instructor/client_vertexai.py and instructor/utils.py. The # type: ignore comment is typically used to suppress type checking errors. Removing it might cause type checking errors if jsonref is not properly typed. However, if the library is now properly typed, this change is fine. It's important to ensure that the removal of # type: ignore does not introduce any type checking issues.

Workflow ID: wflow_xWd3nCujgk3F6tP4


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

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 dcca805 in 28 seconds

More details
  • Looked at 66 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. pyproject.toml:35
  • Draft comment:
    The version for mistralai is inconsistent with the PR description. The description mentions updating to version ^1.0.3, but the file shows ^0.1.8. Please ensure the correct version is specified.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is about a change made in the diff, specifically the version change of mistralai. However, it references the PR description, which I cannot verify. The comment is speculative as it asks to ensure the correct version is specified, which violates the rules. Without access to the PR description, I cannot confirm the discrepancy, so I should assume the comment is not useful.
    I might be missing the context of the PR description, which could validate the comment. However, the rules state not to ask for confirmation or verification, so the comment might still be invalid even if the discrepancy exists.
    Even if the PR description mentions a different version, the comment is speculative and asks for verification, which is against the rules.
    Delete the comment as it is speculative and asks for verification, which is against the rules.

Workflow ID: wflow_dkqbeNPtT3djzLq4


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

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 64111c7 in 13 seconds

More details
  • Looked at 22 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. pyproject.toml:35
  • Draft comment:
    Ensure that the mistralai version update to ^1.0.3 is consistent across all relevant sections in the pyproject.toml. It appears correct here and in other sections like [tool.poetry.group.test-docs.dependencies].
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The PR updates the mistralai dependency version in pyproject.toml from ^0.1.8 to ^1.0.3. This change is consistent across the file, which is good. However, it's important to ensure that all instances of the version update are correct and consistent.

Workflow ID: wflow_gshY1kmo22Zr2BA4


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

Copy link
Contributor

ellipsis-dev bot commented Sep 18, 2024

Skipped PR review on c9461b0 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.


Generated with ❤️ by ellipsis.dev

@ivanleomk
Copy link
Collaborator Author

This PR allows us to make requests to Mistral as seen below

import instructor
from mistralai import Mistral
from pydantic import BaseModel
import os

client = Mistral(api_key=os.getenv("MISTRAL_API_KEY"))

client = instructor.from_mistral(client)


class User(BaseModel):
    name: str
    age: int


resp = client.chat.completions.create(
    model="mistral-large-latest",
    messages=[
        {
            "role": "user",
            "content": "Extract the user information from the following text: John is 25 years old and works as a software engineer.",
        }
    ],
    response_model=User,
)

print(resp)
# > name='John' age=25

@ivanleomk
Copy link
Collaborator Author

This also works for the async version

import asyncio
import instructor
from mistralai import Mistral
from pydantic import BaseModel
import os

client = Mistral(api_key=os.getenv("MISTRAL_API_KEY"))

client = instructor.from_mistral(client, use_async=True)


class User(BaseModel):
    name: str
    age: int


async def main():
    resp = await client.chat.completions.create(
        model="mistral-large-latest",
        messages=[
            {
                "role": "user",
                "content": "Extract the user information from the following text: John is 25 years old and works as a software engineer.",
            }
        ],
        response_model=User,
    )

    print(resp)
    # > name='John' age=25


if __name__ == "__main__":
    asyncio.run(main())

@ivanleomk ivanleomk merged commit a2b108a into main Sep 18, 2024
14 of 15 checks passed
@ivanleomk ivanleomk deleted the mistral-fix branch September 18, 2024 23:52
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