Update image_edit.py#3
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d40f7b5bc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| args.model_path = _normalize_path(args.model_path) | ||
| if not os.path.isdir(args.model_path): | ||
| p.error("--model_path must be an existing directory") |
There was a problem hiding this comment.
The new validation rewrites --model_path to an absolute filesystem path and then requires it to exist as a local directory, which breaks the documented/standard Hugging Face usage where from_pretrained receives a repo ID like inclusionAI/LLaDA2.0-Uni. In that case this script now exits in argument parsing before loading anything, so the CLI no longer works for non-local model paths.
Useful? React with 👍 / 👎.
![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
This pull request significantly refactors the image editing script to improve robustness, including enhanced argument parsing, better memory management, and stricter validation of input data and model outputs. A review comment was provided suggesting a more robust implementation for handling directory paths in the output file path preparation logic to prevent potential issues when users provide directory paths.
| def _prepare_output_path(path): | ||
| path = _normalize_path(path) | ||
| root, ext = os.path.splitext(path) | ||
| if not ext or ext == ".": | ||
| path = (root if root else path) + ".png" | ||
| parent = os.path.dirname(path) | ||
| if parent: | ||
| os.makedirs(parent, exist_ok=True) | ||
| return path |
There was a problem hiding this comment.
The logic for preparing the output path doesn't correctly handle directory paths. If a user provides a path ending in a separator (e.g., --output my_dir/), os.path.splitext will treat my_dir/ as the root, leading to an attempt to save a file named .png inside my_dir. This is likely not the intended behavior.
A more robust implementation would check if the provided path is a directory and append a default filename in that case.
| def _prepare_output_path(path): | |
| path = _normalize_path(path) | |
| root, ext = os.path.splitext(path) | |
| if not ext or ext == ".": | |
| path = (root if root else path) + ".png" | |
| parent = os.path.dirname(path) | |
| if parent: | |
| os.makedirs(parent, exist_ok=True) | |
| return path | |
| def _prepare_output_path(path): | |
| path = _normalize_path(path) | |
| if os.path.isdir(path) or path.endswith(os.path.sep): | |
| path = os.path.join(path, "edited.png") | |
| else: | |
| root, ext = os.path.splitext(path) | |
| if not ext or ext == ".": | |
| path = (root if root else path) + ".png" | |
| parent = os.path.dirname(path) | |
| if parent: | |
| os.makedirs(parent, exist_ok=True) | |
| return path |
1 participant
No description provided.