Changes by create-pull-request action #530
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![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 replaces enum classes with string literals across the infinity client models, focusing on simplifying type definitions while maintaining functionality.
- Replaces
EmbeddingObjectObject,ClassifyResultObject, and other enum classes withLiteraltypes (e.g.,Literal['embedding'],Literal['classify']) in model classes - Updates validation logic in
from_dictmethods to directly check string values instead of using enum comparisons - Removes multiple modality-related enum files (e.g.,
open_ai_embedding_input_audio_modality.py) and replaces with literal types - Adds health check assertion and docstring to
test_colpalifunction invision_client.py - Potential concern: Removal of enum classes could impact code that directly imports these types from the models package
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
19 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings | Greptile
| object_ = cast(Union[Literal["classify"], Unset], d.pop("object", UNSET)) | ||
| if object_ != "classify" and not isinstance(object_, Unset): | ||
| raise ValueError(f"object must match const 'classify', got '{object_}'") |
There was a problem hiding this comment.
style: The error message should include the actual type of the invalid value to help with debugging
There was a problem hiding this comment.
logic: This file has been completely removed but is still imported and used in classify_result.py. This will cause runtime errors when validating the 'object_' field.
| object_ = cast(Union[Literal["model"], Unset], d.pop("object", UNSET)) | ||
| if object_ != "model" and not isinstance(object_, Unset): | ||
| raise ValueError(f"object must match const 'model', got '{object_}'") |
There was a problem hiding this comment.
style: The error message should include the expected type. Consider 'object_ must be "model" or Unset' for better clarity.
There was a problem hiding this comment.
logic: This file is being deleted but ModelInfo class in model_info.py still references 'infinity' as a default value for owned_by. Make sure all references to this enum are properly updated.
There was a problem hiding this comment.
logic: This file is being deleted but ModelInfoObject appears to be used in model_info.py. Verify that all references to this enum are properly handled or removed to prevent runtime errors.
| 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
| 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}'") | |
| modality = d.pop("modality", UNSET) | |
| if modality != "text" and not isinstance(modality, Unset): | |
| raise ValueError(f"modality must match const 'text', got '{modality}'") | |
| modality = cast(Union[Literal["text"], Unset], modality) |
| object_ = cast(Union[Literal["list"], Unset], d.pop("object", UNSET)) | ||
| if object_ != "list" and not isinstance(object_, Unset): | ||
| raise ValueError(f"object must match const 'list', got '{object_}'") |
There was a problem hiding this comment.
logic: The validation logic here is more strict than before - it will now raise an error for any value other than 'list', while previously it would accept any value that could be converted to an enum member. Make sure this aligns with the API's behavior.
There was a problem hiding this comment.
logic: This file's complete removal may break code that imports OpenAIEmbeddingResultObject. Check all imports and usages across the codebase, particularly in open_ai_embedding_result.py which likely depends on this enum.
There was a problem hiding this comment.
logic: This file is being completely removed but is still imported in init.py and may be used by other parts of the codebase. Need to verify all dependencies are updated or confirm this removal is intentional.
| object_ = cast(Union[Literal["rerank"], Unset], d.pop("object", UNSET)) | ||
| if object_ != "rerank" and not isinstance(object_, Unset): | ||
| raise ValueError(f"object must match const 'rerank', got '{object_}'") |
There was a problem hiding this comment.
style: The cast and validation logic here could potentially allow non-string values through if they match 'rerank'. Consider adding an explicit type check for string values.
| object_ = cast(Union[Literal["rerank"], Unset], d.pop("object", UNSET)) | |
| if object_ != "rerank" and not isinstance(object_, Unset): | |
| raise ValueError(f"object must match const 'rerank', got '{object_}'") | |
| object_ = cast(Union[Literal["rerank"], Unset], d.pop("object", UNSET)) | |
| if not isinstance(object_, Unset) and (not isinstance(object_, str) or object_ != "rerank"): | |
| raise ValueError(f"object must be string 'rerank', got '{type(object_).__name__}' with value '{object_}'") |
Automated changes by create-pull-request GitHub action