修复_load_imgs中isinstance(img_content,InputType)错误#140
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a TypeError in the _load_imgs method where isinstance(img_content, InputType) fails because subscripted generics cannot be used with class and instance checks. The fix introduces early type validation using get_args() and simplifies the list handling logic.
- Adds early type validation in the
__call__method to check input types before processing - Simplifies
_load_imgsto only check if input is a list rather than usingisinstancewithInputType - Imports
get_argsfrom typing module to enable proper type checking of Union types
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| for img_content in img_contents: | ||
| if not isinstance(img_content, get_args(InputType)): | ||
| type_names = ", ".join([t.__name__ for t in get_args(InputType)]) | ||
| actual_type = ( | ||
| type(img_content).__name__ if img_content is not None else "None" | ||
| ) | ||
| raise TypeError( | ||
| f"Type Error: Expected input of type [{type_names}], but received '{img_content}' of type {actual_type}." |
There was a problem hiding this comment.
The error message includes the actual value '{img_content}' which could expose sensitive data or create overly verbose output for large objects. Consider removing the value from the error message and only showing the type information.
| for img_content in img_contents: | |
| if not isinstance(img_content, get_args(InputType)): | |
| type_names = ", ".join([t.__name__ for t in get_args(InputType)]) | |
| actual_type = ( | |
| type(img_content).__name__ if img_content is not None else "None" | |
| ) | |
| raise TypeError( | |
| f"Type Error: Expected input of type [{type_names}], but received '{img_content}' of type {actual_type}." | |
| f"Type Error: Expected input of type [{type_names}], but received type {actual_type}." |
| img_contents = ( | ||
| [img_content] if isinstance(img_content, InputType) else img_content | ||
| ) | ||
| img_contents = img_content if isinstance(img_content, list) else [img_content] |
There was a problem hiding this comment.
This change breaks the original contract where _load_imgs accepted Union[Sequence[InputType], InputType] but now only checks for list. Other sequence types like tuples would incorrectly be wrapped in a list. Consider using isinstance(img_content, (list, tuple)) or checking against Sequence to maintain the original API contract.
| img_contents = img_content if isinstance(img_content, list) else [img_content] | |
| img_contents = ( | |
| img_content | |
| if isinstance(img_content, Sequence) and not isinstance(img_content, (str, bytes)) | |
| else [img_content] | |
| ) |
原来的代码,判断类型会报错 TypeError: Subscripted generics cannot be used with class and instance checks
修复如下: