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

Initial support for attachments for multi-modal models #590

Merged
merged 18 commits into from
Oct 28, 2024
Merged

Conversation

simonw
Copy link
Owner

@simonw simonw commented Oct 28, 2024

@simonw simonw added the enhancement New feature or request label Oct 28, 2024
@simonw
Copy link
Owner Author

simonw commented Oct 28, 2024

Getting a bunch of mypy failures:

CleanShot 2024-10-28 at 15 08 48@2x

@simonw
Copy link
Owner Author

simonw commented Oct 28, 2024

% just mypy
llm/models.py:87: error: Incompatible types in assignment (expression has type "dict[Never, Never]", variable has type "Options")  [assignment]
llm/models.py:118: error: Incompatible default for argument "attachments" (default has type "None", argument has type "Attachment")  [assignment]
llm/models.py:118: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
llm/models.py:118: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
llm/models.py:374: error: Need type annotation for "attachment_types" (hint: "attachment_types: set[<type>] = ...")  [var-annotated]
llm/models.py:400: error: Incompatible default for argument "attachments" (default has type "None", argument has type "Attachment")  [assignment]
llm/models.py:400: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
llm/models.py:400: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
llm/models.py:410: error: Item "Attachment" of "Attachment | list[Never]" has no attribute "__iter__" (not iterable)  [union-attr]
Found 5 errors in 1 file (checked 15 source files)
error: Recipe `mypy` failed on line 28 with exit code 1

@simonw
Copy link
Owner Author

simonw commented Oct 28, 2024

Tried this:

files-to-prompt . -c -e py | llm -m claude-3.5-sonnet --system 'llm/models.py:87: error: Incompatible types in assignment (expression has type "dict[Never, Never]", variable has type "Options")  [assignment]
llm/models.py:118: error: Incompatible default for argument "attachments" (default has type "None", argument has type "Attachment")  [assignment]
llm/models.py:118: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
llm/models.py:118: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
llm/models.py:374: error: Need type annotation for "attachment_types" (hint: "attachment_types: set[<type>] = ...")  [var-annotated]
llm/models.py:400: error: Incompatible default for argument "attachments" (default has type "None", argument has type "Attachment")  [assignment]
llm/models.py:400: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
llm/models.py:400: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
llm/models.py:410: error: Item "Attachment" of "Attachment | list[Never]" has no attribute "__iter__" (not iterable)  [union-attr]'

Based on the code, I'll help fix the mypy errors in llm/models.py. Here are the fixes:

  1. For the incompatible types in assignment error:
# Change this:
self.options = options or {}

# To this:
if options is None:
    self.options = self.Options()
else:
    self.options = options
  1. For the attachment type issues, add type annotations and Optional:
from typing import Optional, List

class Prompt:
    def __init__(
        self,
        prompt: str,
        model: "Model",
        *,
        attachments: Optional[List[Attachment]] = None,
        system: Optional[str] = None,
        prompt_json: Optional[str] = None, 
        options: Optional[Dict[str, Any]] = None
    ):
        self.prompt = prompt
        self.model = model
        self.attachments = list(attachments or [])
        self.system = system
        self.prompt_json = prompt_json
        if options is None:
            self.options = self.Options() 
        else:
            self.options = options
  1. For the attachment_types error:
class Model(ABC, _get_key_mixin):
    # Add type annotation
    attachment_types: Set[str] = set()
  1. For the union attribute error when checking attachments:
def prompt(
    self,
    prompt: str,
    *,
    attachments: Optional[List[Attachment]] = None,
    system: Optional[str] = None,
    stream: bool = True,
    **options
):
    # Validate attachments
    if attachments and not self.attachment_types:
        raise ValueError(
            "This model does not support attachments, but some were provided"
        ) 
    
    # Use proper list typing
    attachments_list = list(attachments or [])
    
    for attachment in attachments_list:
        attachment_type = attachment.resolve_type()
        if attachment_type not in self.attachment_types:
            raise ValueError(
                f"This model does not support attachments of type '{attachment_type}', only {', '.join(self.attachment_types)}"
            )

Key changes made:

  1. Added proper type annotations for Options and attachments arguments
  2. Fixed initialization of self.options to use proper Options type
  3. Added Set[str] type annotation for attachment_types class variable
  4. Fixed attachments handling to properly handle Optional[List[Attachment]]
  5. Added proper type hints for method arguments

This should resolve all the mypy type checking errors while maintaining the existing functionality.

Let me know if you would like me to explain any of the changes in more detail!

@simonw simonw merged commit 758ff9a into main Oct 28, 2024
61 checks passed
@simonw simonw deleted the attachments branch October 28, 2024 22:41
simonw added a commit that referenced this pull request Oct 28, 2024
simonw added a commit that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant