docs: correct stated defaults in Chroma pipeline docstrings - #14578
Conversation
|
Hi @iridescentWen, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice. Once the PR links an issue (or gets the |
Seven `defaults to X` entries in the three Chroma pipelines' __call__ docstrings name a value the signature does not use, so copying the documented number reproduces different behavior than leaving the argument out. Values taken from each __call__ signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1921ab3 to
3ccd5b9
Compare
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Fixes #14668
Third docs PR after #14345 and #14541 (both merged).
What's wrong
The three Chroma pipelines'
__call__docstrings statedefaults to Xfor seven parameters wherethe signature uses a different value. Anyone who reads the docstring and passes the documented number
explicitly gets different behavior than omitting the argument — which is the opposite of what a
stated default is for.
Every value below was read from the corresponding
__call__signature via AST, not inferred:__call__pipeline_chroma.py:589num_inference_stepspipeline_chroma.py:589guidance_scalepipeline_chroma_img2img.py:648guidance_scalepipeline_chroma_inpainting.py:766num_inference_stepspipeline_chroma_inpainting.py:766guidance_scalepipeline_chroma_inpainting.py:766strengthpipeline_chroma_inpainting.py:766max_sequence_lengthThe pattern looks like the docstrings were carried between the three pipelines (and from a Flux-style
ancestor) while the signatures were retuned per pipeline. Note the internal evidence that
defaults toreally is meant to mirror the literal signature default here: in the samepipeline_chroma.pydocstring,num_images_per_prompt (defaults to 1)andmax_sequence_length (defaults to 512)both match their signature exactly — only these seven drifted.Docstrings only, no behavior change.
Correct ones left alone
Not everything the scan surfaced was wrong, and I did not touch what already agrees:
pipeline_chroma_img2img.py'snum_inference_steps(35) andstrength(0.9) both match, as doesmax_sequence_length(512) inpipeline_chroma.pyandpipeline_chroma_img2img.py.How I found it, and one thing I got wrong on the way
An AST pass comparing each documented
defaults to <literal>against the signature's actual literaldefault. It reports only when both sides are comparable literals, and deliberately skips two classes
that are not defects: a
Nonesignature default whose docstring names the value resolved in thebody (intentional), and prose defaults like "defaults to the model's current device".
Worth flagging because it nearly produced a wrong entry above: my first pass used a file-wide grep for
max_sequence_length:and picked upencode_prompt's signature (= 512) instead of__call__'s(
= 256), which would have made the inpainting row look correct. The table is built fromper-function AST lookups for that reason.
Validation
Re-running the scan over
src/diffusers/pipelines/chromaafter the change reports 0 mismatches.git diff --stat: 3 files, +7/-7.Self-review (per CONTRIBUTING)
Ran the
.ai/skills/self-reviewrubric against.ai/review-rules.md:# Copied from: all three__call__methods checked individually — none carries a# Copied fromheader (the markers in these files sit on helpers likeencode_promptand_pack_latents), so editing the three docstrings directly is correct rather than fixing an upstreamsource and running
make fix-copies.utils/check_copies.pyis outside my sparse checkout, so thisis the manual equivalent; CI's consistency check will confirm.
docs/page repeats these numbers(checked
docs/source/en/api/pipelines/chroma.md).Not claimed
The same scan finds ~190 more
defaults tomismatches elsewhere insrc/diffusers/, heavilyclustered in other pipeline families (
num_inference_steps 50 → 28in 13 files,guidance_scale 7.5 → 5.0in 12, and so on). I kept this PR to one pipeline family rather thansending a repo-wide sweep. If you'd like the rest, tell me whether you prefer them per-family or as
one batch and I'll follow up — and if you'd rather this were enforced mechanically instead, that scan
could become a
utils/check.🤖 Written with Claude Code. All seven signature defaults were read
programmatically and the correct-already entries were verified rather than assumed.