update input_info #774
update input_info #774
Conversation
Summary of ChangesHello @helloyongyang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the way input parameters are handled throughout the system, moving from a direct initialization method to a more flexible two-step process involving empty object initialization followed by data population. This change enhances the modularity and extensibility of input configuration, introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a significant refactoring of how input information is handled across various modules. The previous set_input_info function has been replaced by a more modular approach using init_empty_input_info for object creation and update_input_info_from_dict or update_input_info_from_object for populating data. This change enhances modularity and maintainability. Additionally, a resize_mode field has been added to all InputInfo dataclasses, and the default aspect_ratio in infer.py has been made empty, indicating more flexible handling of image dimensions. The ASPECT_RATIO_MAP constant was removed from z_image_runner.py, and the logic for determining custom shapes was centralized and enhanced in both qwen_image_runner.py and z_image_runner.py to include default aspect ratios and size constraints.
| parser.add_argument("--return_result_tensor", action="store_true", help="Whether to return result tensor. (Useful for comfyui)") | ||
| parser.add_argument("--target_shape", nargs="+", default=[], help="Set return video or image shape") | ||
| parser.add_argument("--aspect_ratio", type=str, default="16:9") | ||
| parser.add_argument("--aspect_ratio", type=str, default="") |
There was a problem hiding this comment.
Changing the default aspect_ratio from "16:9" to an empty string "" could lead to unexpected behavior in parts of the system that rely on a non-empty default. If no aspect_ratio is explicitly provided, an empty string might not be a valid or expected value for downstream processing, potentially causing errors or incorrect aspect ratio calculations. It's generally safer to maintain a valid default or ensure explicit handling of the empty string.
| width, height = int(width * scale), int(height * scale) | ||
| logger.warning(f"Custom shape is too large, scaled to {width}x{height}") | ||
| width, height = max(width, min_size), max(height, min_size) | ||
| logger.info(f"Qwen Image Runner got custom shape: {width}x{height}") |
There was a problem hiding this comment.
The logger.info message on this line incorrectly refers to "Qwen Image Runner". This appears to be a copy-paste error and should be corrected to "Z Image Runner" for accuracy and clarity in the logs.
| logger.info(f"Qwen Image Runner got custom shape: {width}x{height}") | |
| logger.info(f"Z Image Runner got custom shape: {width}x{height}") |
|
|
||
| aspect_ratio = self.input_info.aspect_ratio if self.input_info.aspect_ratio else self.config.get("aspect_ratio", None) | ||
| if aspect_ratio in as_maps: | ||
| logger.info(f"Qwen Image Runner got aspect ratio: {aspect_ratio}") |
There was a problem hiding this comment.
The logger.info message on this line incorrectly refers to "Qwen Image Runner". This appears to be a copy-paste error and should be corrected to "Z Image Runner" for accuracy and clarity in the logs.
| logger.info(f"Qwen Image Runner got aspect ratio: {aspect_ratio}") | |
| logger.info(f"Z Image Runner got aspect ratio: {aspect_ratio}") |
|
|
||
| config = set_config(self) | ||
| print_config(config) | ||
| print(config) |
There was a problem hiding this comment.
Replacing the dedicated print_config(config) function with a generic print(config) removes the structured logging and filtering capabilities provided by print_config (e.g., handling parallel configurations and removing device_mesh). This can lead to less informative or potentially verbose logging, and might expose internal configuration details that were previously suppressed.
| print(config) | |
| print_config(config) |
No description provided.