Changes by create-pull-request action#571
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR makes significant changes to the model definitions in the infinity client library, primarily focusing on enum simplification. Here's a summary of the key changes:
- Replaces multiple enum classes with Python's
Literaltypes for type-safe string constants across model files - Removes 8 enum definition files while maintaining their functionality through literal type constraints
- Updates validation logic in model classes to work with literal string values instead of enum comparisons
- Adds a basic test function
test_colpali()in vision_client.py demonstrating API usage
Key points to review:
- Verify that all dependent code using these enums has been properly updated to use the new literal types
- Check that error handling and validation logic remains robust after removing enum-based type checking
- Consider adding more comprehensive tests beyond the basic example in vision_client.py
- Ensure documentation is updated to reflect the removal of enum-based constants
- Review the automated nature of these changes for unintended side effects
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
19 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile
| from .classify_result import ClassifyResult | ||
| from .classify_result_object import ClassifyResultObject | ||
| from .embedding_encoding_format import EmbeddingEncodingFormat | ||
| from .embedding_object import EmbeddingObject |
There was a problem hiding this comment.
logic: Removing EmbeddingObjectObject may break code that relies on the 'embedding' literal type checking in embedding_object.py
| from .open_ai_embedding_input_audio import OpenAIEmbeddingInputAudio | ||
| from .open_ai_embedding_input_audio_modality import OpenAIEmbeddingInputAudioModality | ||
| from .open_ai_embedding_input_image import OpenAIEmbeddingInputImage | ||
| from .open_ai_embedding_input_image_modality import OpenAIEmbeddingInputImageModality | ||
| from .open_ai_embedding_input_text import OpenAIEmbeddingInputText |
There was a problem hiding this comment.
logic: Removing OpenAIEmbeddingInput modality enums could affect type validation in the corresponding input classes
There was a problem hiding this comment.
logic: Removing this file may break imports in other modules that use ModelInfoObject. Check all references to this class across the codebase.
There was a problem hiding this comment.
logic: Removing this file may break code that depends on EmbeddingObjectObject. Check that all references to this enum are properly handled in the rest of the codebase.
There was a problem hiding this comment.
logic: Removing this enum class will break functionality in OpenAIEmbeddingInputImage and potentially other classes that depend on this modality type. Either keep this file or provide an alternative implementation.
| modality = cast(Union[Literal["text"], Unset], d.pop("modality", UNSET)) | ||
| if modality != "text" and not isinstance(modality, Unset): | ||
| raise ValueError(f"modality must match const 'text', got '{modality}'") |
There was a problem hiding this comment.
logic: validation should happen before casting to avoid potential type errors if invalid value is passed
There was a problem hiding this comment.
logic: This file's removal may break OpenAIEmbeddingInputText and other components that import OpenAIEmbeddingInputTextModality. Verify all dependencies are updated.
| embeddings, total_tokens = future.result() | ||
| print(embeddings, total_tokens) |
There was a problem hiding this comment.
style: Replace print statement with assertions to verify embedding dimensions and token count
| embeddings, total_tokens = future.result() | |
| print(embeddings, total_tokens) | |
| embeddings, total_tokens = future.result() | |
| assert isinstance(embeddings, list) and len(embeddings) == 1 | |
| assert embeddings[0].shape[-1] == colpali.hidden_dim | |
| assert total_tokens > 0 |
1 participant
Automated changes by create-pull-request GitHub action