fix: Make ColorTransfer node ref_image mandatory (CORE-140)#13691
fix: Make ColorTransfer node ref_image mandatory (CORE-140)#13691alexisrolland merged 2 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughThe 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
comfy_extras/nodes_post_processing.py (1)
841-842:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
image_ref is Noneis now unreachable dead code.Since this PR removes
optional=Truefrom theimage_refinput, the execution framework will always provide a tensor here. Theimage_ref is Nonebranch can never be taken and should be cleaned up.🧹 Proposed cleanup
- if strength == 0 or image_ref is None: + if strength == 0: return io.NodeOutput(image_target)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_extras/nodes_post_processing.py` around lines 841 - 842, The condition "image_ref is None" in the post-processing early-return is dead code now that image_ref is always provided; in the function containing the line "if strength == 0 or image_ref is None: return io.NodeOutput(image_target)" remove the "or image_ref is None" check so the branch only checks strength (i.e., change to "if strength == 0: return io.NodeOutput(image_target)"), and run/adjust any related tests or callers expecting a possible None value for image_ref; keep references to io.NodeOutput and image_target unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@comfy_extras/nodes_post_processing.py`:
- Around line 841-842: The condition "image_ref is None" in the post-processing
early-return is dead code now that image_ref is always provided; in the function
containing the line "if strength == 0 or image_ref is None: return
io.NodeOutput(image_target)" remove the "or image_ref is None" check so the
branch only checks strength (i.e., change to "if strength == 0: return
io.NodeOutput(image_target)"), and run/adjust any related tests or callers
expecting a possible None value for image_ref; keep references to io.NodeOutput
and image_target unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31c5ea93-4eb8-4508-8020-612652e9b86c
📒 Files selected for processing (1)
comfy_extras/nodes_post_processing.py
|
After discussing with @rattus128 we aligned that it would be preferable to make |
The
ColorTransfernode has aref_imageinput which is optional but when it is not provided, the node triggers the error:ref_imageinput to be optionalBefore:
After: